Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot change revisions_to_keep for a storage pool #3256

Closed
na-- opened this issue Oct 29, 2017 · 23 comments
Closed

Cannot change revisions_to_keep for a storage pool #3256

na-- opened this issue Oct 29, 2017 · 23 comments

Comments

@na--
Copy link

na-- commented Oct 29, 2017

Qubes OS version:

R4.0 RC2

Affected TemplateVMs:

none (dom0 issue)


Steps to reproduce the behavior:

Try to change the number of revisions_to_keep for a storage pool with the qvm-pool command

Expected behavior:

That there is a relatively easy way to modify (or at least increase) the number of revisions kept

Actual behavior:

None such exists - probably editing qubes.xml directly will do the job, but not sure if that won't mess something up.

General notes:

I am not sure that it's a good idea to mess with storage pool options after a pool is created, too many things can go wrong. The exception I think is increasing revisions_to_keep, even decreasing it can be problematic if not handled correctly. So handling such a special case in qvm-pool might not be the best way to proceed...
A workaround that I think could be used right now looks like this:

  • create a new storage pool that is identical to the last with the last (same folder/volume group&thin pool), but with different revisions_to_keep value
  • clone vms from the old pool to the new one (with a slightly changed name)
  • delete the original vms
  • clone the new vms in the same pool with the origin names
  • delete the vms with the slightly changed names

It's a bit of a mess, especially when cloning templates :) Is there an easier way to accomplish this?


Related issues:

None that I could find

@marmarek
Copy link
Member

I'm already working on this. Better not change this (or any other) value manually in qubes.xml, because driver may want to apply some constrains, or do some conversion when changing it.

@andrewdavidwong andrewdavidwong added this to the Release 4.0 milestone Oct 29, 2017
@marmarek marmarek self-assigned this Nov 6, 2017
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Nov 7, 2017
Do not check for accepted value only in constructor, do that in property
setter. This will allow enforcing the limit regardless of how the value
was set.

This is preparation for dynamic revisions_to_keep change.

QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Nov 7, 2017
Do not check for accepted value only in constructor, do that in property
setter. This will allow enforcing the limit regardless of how the value
was set.

This is preparation for dynamic revisions_to_keep change.

QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Nov 7, 2017
This one pool/volume property makes sense to change dynamically. There
may be more such properties, but lets be on the safe side and take
whitelist approach - allow only selected (just one for now), instead of
blacklisting any harmful ones.

QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Nov 7, 2017
Do not check for accepted value only in constructor, do that in property
setter. This will allow enforcing the limit regardless of how the value
was set.

This is preparation for dynamic revisions_to_keep change.

QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Nov 7, 2017
This one pool/volume property makes sense to change dynamically. There
may be more such properties, but lets be on the safe side and take
whitelist approach - allow only selected (just one for now), instead of
blacklisting any harmful ones.

QubesOS/qubes-issues#3256
@tlaurion
Copy link
Contributor

tlaurion commented Feb 7, 2018

@marmarek is it supposed to work? In 4.0 rc4, qvm-pool -i lvm -o revisions_to_keep=3 doesn't have any effect.

@marmarek
Copy link
Member

qvm-pool -i just display information, it isn't supposed to change anything. This ticket is about adding an option for that... For now the backend side (Admin API) is already done.

@tlaurion
Copy link
Contributor

tlaurion commented Feb 15, 2018

@marmarek I tried to change manually qubes.xml (revisions_to_keep=3) with same result.

Any manual operations to make the backend take it in consideration atm?

I'm looking into that feature to make some changes to dom0 to update Qubes Network Server internals, and revert easily when messing up.

@rustybird
Copy link

I tried to change manually qubes.xml (revisions_to_keep=3) with same result.

Any manual operations to make the backend take it in consideration atm?

Setting revisions_to_keep in the individual <volume> elements (not just once in <pool>) looks like it would work for the lvm_thin driver. But I haven't tried it.

@marmarek
Copy link
Member

marmarek commented Feb 15, 2018

You can also use Admin API directly, until proper UI get implemented:

echo -n 2 | qubesd-query dom0 admin.vm.volume.Set.revisions_to_keep <VMNAME> <VOLUME>

marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 18, 2018
marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 18, 2018
This allows to get and set volumes properties.

Fixes QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 18, 2018
This allows to get and set volumes properties.

Fixes QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 18, 2018
This allows to get and set volumes properties.

Fixes QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Mar 19, 2018
Since Volume.is_outdated() is a method, not a property, add a function
for handling serialization. And at the same time, fix None serialization
(applicable to 'source' property).

QubesOS/qubes-issues#3256
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Mar 19, 2018
Since Volume.is_outdated() is a method, not a property, add a function
for handling serialization. And at the same time, fix None serialization
(applicable to 'source' property).

QubesOS/qubes-issues#3256
@qubesos-bot
Copy link

Automated announcement from builder-github

The package python2-qubesadmin-4.0.16-0.1.fc25 has been pushed to the r4.0 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-admin-client has been pushed to the r4.0 testing repository for the CentOS centos7 template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package python2-qubesadmin-4.0.17-0.1.fc25 has been pushed to the r4.0 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-admin-client (including package python2-qubesadmin-4.0.17-0.1.fc26) has been pushed to the r4.0 testing repository for the Fedora template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-core-admin-client_4.0.17-1+deb9u1 has been pushed to the r4.0 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing stretch-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-admin-client has been pushed to the r4.0 testing repository for the CentOS centos7 template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-core-admin-client_4.0.17-1+deb9u1 has been pushed to the r4.0 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-admin-client has been pushed to the r4.0 stable repository for the Fedora centos7 template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-admin-client (including package python2-qubesadmin-4.0.17-0.1.fc26) has been pushed to the r4.0 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package python2-qubesadmin-4.0.17-0.1.fc25 has been pushed to the r4.0 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment