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

qemu-guest-agent: init module #39099

Merged
merged 1 commit into from May 9, 2018
Merged

qemu-guest-agent: init module #39099

merged 1 commit into from May 9, 2018

Conversation

teto
Copy link
Member

@teto teto commented Apr 18, 2018

Motivation for this change

Not finished but I could use feedback on what to provide as an option or not. The Qemu guest agent allows out of band communications with Qemu VMs and allows for fancy things.

Ref:
NixOS/nixops#922
#34722

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

enable = mkOption {
type = types.bool;
default = false;
description = "Whether to enable the dspam spam filter.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, fixed locally

SUBSYSTEM=="virtio-ports", ATTR{name}=="org.qemu.guest_agent.0", TAG+="systemd" ENV{SYSTEMD_WANTS}="qemu-guest-agent.service"
'';

environment.systemPackages = [ pkgs.qemu ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic:

  1. It makes the guest depend on full qemu although it just needs the guest-agent.
  2. It's a hard reference to standard qemu. We have several variants like qemu_test and qemu_kvm, and it is difficult to tell from the guest config which one the host will use to run the vm. So this may build/download an additional qemu version ... just for the guest-agent.

I think the cleanest solution is to create a separate small qemu-guest-agent package (as many other distros do), no variants needed for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, in fact I had started writing the package qemu-agent before realizing it was already in the package. I am not sure how easy it is to generate just the Qemu agent, I wouldn't mind someone else doing it (or I can try later but I am short on time this month).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try and package qemu-guest-agent, wanted to do that anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a separate package turned out to be rather messy, so I put the quest agent in a separate output in #39212. With this merged, you can just use qemu.ga in your module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the agent
Regarding your comment

It's a hard reference to standard qemu. We have several variants like qemu_test and qemu_kvm, and it is difficult to tell from the guest config which one the host will use to run the vm. So this may build/download an additional qemu version ... just for the guest-agent.

How do you propose to solve it ?
Should I use another option qemuPackage ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I use another option qemuPackage ?

You could do that but not sure I'm not sure that's necessary. If you use qemu.ga in the guest config, it should only install the small ga output.

@@ -343,6 +343,15 @@ in
(<literal>virtio</literal> or <literal>scsi</literal>).
'';
};

guestAgent =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other options I suggest calling this guestAgent.enable or useGuestAgent.
Also, spelling should be unified beween guestAgent for one option and qemu-guest-agent for the other.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ig uess I'll follow the pattern virtualboxGuest, hypvGuest with qemuGuest (https://nixos.org/nixos/options.html#guest)

@teto
Copy link
Member Author

teto commented Apr 26, 2018

@xeji I updated the PR hoping that's cleaner.

@teto teto changed the title [WIP] qemu-guest-agent: init module [RFC] qemu-guest-agent: init module Apr 26, 2018
enable = mkOption {
type = types.bool;
default = false;
description = "Whether to enable the dspam spam filter.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you fix this before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I fucked up the merge of the changes from different branches sorry


systemd.services.qemu-guest-agent = {
description = "Run the QEMU Guest Agent";
path = [ pkgs.qemu ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? you have the full path to the binary in ExecStart=

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope removing it

description = "Run the QEMU Guest Agent";
path = [ pkgs.qemu ];
serviceConfig = {
ExecStart = "${pkgs.kvm}/bin/qemu-ga";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to use pkgs.qemu.ga or pkgs.kvm.ga here as discussed earlier. Should be consistent with what you use in environment.systemPackages above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I could get rid of the environment.systemPackages too

@teto teto force-pushed the qemu_agent branch 2 times, most recently from 9407da6 to 514d8f2 Compare April 26, 2018 10:53
@teto
Copy link
Member Author

teto commented Apr 26, 2018

Once again xD ?

systemd.services.qemu-guest-agent = {
description = "Run the QEMU Guest Agent";
serviceConfig = {
ExecStart = "${pkgs.kvm}/bin/qemu-ga";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ${pkgs.kvm.ga}/bin/qemu-ga. The guest needs only the guest agent, not the full package.

@teto teto force-pushed the qemu_agent branch 2 times, most recently from 9717c8e to ca06041 Compare April 27, 2018 00:33
Allow out of band communication between qemu VMs and the host.
Useful to retrieve IPs of VMs from the host (for instance when libvirt can't analyze
DHCP requests because VMs are configured with static addresses or when
there is connectivity default).
@teto
Copy link
Member Author

teto commented May 1, 2018

right; fixed.

@teto teto changed the title [RFC] qemu-guest-agent: init module [RDY] qemu-guest-agent: init module May 1, 2018
@teto
Copy link
Member Author

teto commented May 8, 2018

@xeji is that ok (sorry for the repeated mistakes) ?

@xeji
Copy link
Contributor

xeji commented May 8, 2018

Looks good. Let me try it locally before merging.

@xeji xeji changed the title [RDY] qemu-guest-agent: init module qemu-guest-agent: init module May 9, 2018
@xeji xeji merged commit 3d6f752 into NixOS:master May 9, 2018
@teto teto deleted the qemu_agent branch May 10, 2018 02:20
@kvtb
Copy link
Contributor

kvtb commented Sep 26, 2023

Starting via udev (on device appearance) results in the service does not start on switch-to-configuration switch being added to the configuration.

It may have a sense to run the service always, using casual wantedBy = [ "multi-user.target" ]; instead of udev

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

Successfully merging this pull request may close these issues.

None yet

6 participants