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

Consider to make (some of) our default Salt formulas oneshot instead of permanent #2173

Closed
rootkovska opened this Issue Jul 14, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@rootkovska
Member

rootkovska commented Jul 14, 2016

E.g. sys-net-with-usb.sls defines the default Net/USB VM in the following way:

  qvm.prefs:
    - name: {{ vmname }}
    - pcidevs: {{ salt['grains.get']('pci_net_devs', []) + salt['grains.get']('pci_usb_devs', []) }}
    - pci_strictreset: False
    - require:
      - sls: qvm.sys-net

This means the list of PCI devices is strictly defined. Now, imagine a user would like to add an additional PCI devices to be also handled by this VM (a typical example for most laptops would be an SD card reader, which often is exposed as a PCIe device). Nothing prevents the user from doing that. However, once this is done, the execution of highstate action would always result in an error, because Salt would detect the VM has a different property (i.e. a different list of PCI devs), and would attempt to correct this, but qvm-pci would, of course, fail, because the VM would be running.

Even more problematic, however, this would be if the VM was not running -- in that case the Salt stack would silently override the user's decision to have this particular additional device sandboxed.

I think most (all) states relating to the creation of VMs, especially by firstboot, should be oneshot. This is unlike some other states, such as e.g. for keeping the system up to date.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 14, 2016

Member

We can simply disable them after applying in firstboot (by appropriate qubesctl top.disable).
Or even apply them with state.sls instead of enabling them and calling state.highstate, but this might get more complex when in-VM configuration will be involved (which isn't in firstboot yet).

Other ideas like having modules being "oneshot" itself seems fundamentally wrong, because that would negate declarative approach (would result in "do X", instead of "have system in state X").

Member

marmarek commented Jul 14, 2016

We can simply disable them after applying in firstboot (by appropriate qubesctl top.disable).
Or even apply them with state.sls instead of enabling them and calling state.highstate, but this might get more complex when in-VM configuration will be involved (which isn't in firstboot yet).

Other ideas like having modules being "oneshot" itself seems fundamentally wrong, because that would negate declarative approach (would result in "do X", instead of "have system in state X").

@rootkovska

This comment has been minimized.

Show comment
Hide comment
@rootkovska

rootkovska Jul 16, 2016

Member

Yeah, top.disable afer first application sounds good. Just: how we gonna now when to disable it? So that we don't do that too early?

Member

rootkovska commented Jul 16, 2016

Yeah, top.disable afer first application sounds good. Just: how we gonna now when to disable it? So that we don't do that too early?

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 16, 2016

Member

Just: how we gonna now when to disable it?

You've said this: after first application (qubesctl state.highstate run).

Member

marmarek commented Jul 16, 2016

Just: how we gonna now when to disable it?

You've said this: after first application (qubesctl state.highstate run).

@rootkovska

This comment has been minimized.

Show comment
Hide comment
@rootkovska

rootkovska Jul 16, 2016

Member

On Sat, Jul 16, 2016 at 06:34:58AM -0700, Marek Marczykowski-Górecki wrote:

Just: how we gonna now when to disable it?

You've said this: after first application (qubesctl state.highstate run).

Ok, nevermind, I forgot this command was synchronous. So, yeah, no problem.

Member

rootkovska commented Jul 16, 2016

On Sat, Jul 16, 2016 at 06:34:58AM -0700, Marek Marczykowski-Górecki wrote:

Just: how we gonna now when to disable it?

You've said this: after first application (qubesctl state.highstate run).

Ok, nevermind, I forgot this command was synchronous. So, yeah, no problem.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 16, 2016

Member

Ideally we should write the states the way to not interfere with later
user changes - for example instead of overriding PCI devices list, state
should only ensure that particular devices are assigned, but not remove
others.

But it's too late to have it for Qubes 3.2 (our current salt module do
not support this).

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Member

marmarek commented Jul 16, 2016

Ideally we should write the states the way to not interfere with later
user changes - for example instead of overriding PCI devices list, state
should only ensure that particular devices are assigned, but not remove
others.

But it's too late to have it for Qubes 3.2 (our current salt module do
not support this).

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 19, 2016

Member

Automated announcement from builder-github

The package pykickstart-2.13-3.fc23 has been pushed to the r3.2 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

Member

marmarek commented Jul 19, 2016

Automated announcement from builder-github

The package pykickstart-2.13-3.fc23 has been pushed to the r3.2 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

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 28, 2016

Member

Automated announcement from builder-github

The package pykickstart-2.13-3.fc23 has been pushed to the r3.2 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

Member

marmarek commented Jul 28, 2016

Automated announcement from builder-github

The package pykickstart-2.13-3.fc23 has been pushed to the r3.2 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