[libvirt] PATCH: Pass -name argument to QEMU

This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore. src/qemu_conf.c | 19 ++++++++++++++----- src/qemu_conf.h | 5 +++-- tests/qemuxml2argvdata/qemuxml2argv-minimal.args | 2 +- tests/qemuxml2argvtest.c | 23 +++++++++++------------ 4 files changed, 29 insertions(+), 20 deletions(-) Dan. diff -r aab96c0c6974 src/qemu_conf.c --- a/src/qemu_conf.c Fri May 09 19:24:37 2008 -0400 +++ b/src/qemu_conf.c Fri May 09 19:30:26 2008 -0400 @@ -492,10 +492,12 @@ *flags |= QEMUD_CMD_FLAG_KQEMU; if (strstr(help, "-no-reboot")) *flags |= QEMUD_CMD_FLAG_NO_REBOOT; - if (strstr(help, "\n-drive")) - *flags |= QEMUD_CMD_FLAG_DRIVE_OPT; + if (strstr(help, "-name")) + *flags |= QEMUD_CMD_FLAG_NAME; + if (strstr(help, "-drive")) + *flags |= QEMUD_CMD_FLAG_DRIVE; if (strstr(help, "boot=on")) - *flags |= QEMUD_CMD_FLAG_DRIVE_BOOT_OPT; + *flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; if (*version >= 9000) *flags |= QEMUD_CMD_FLAG_VNC_COLON; ret = 0; @@ -2348,6 +2350,7 @@ len = 1 + /* qemu */ 2 + /* machine type */ disableKQEMU + /* Disable kqemu */ + (vm->qemuCmdFlags & QEMUD_CMD_FLAG_NAME ? 2 : 0) + /* -name XXX */ 2 * vm->def->ndisks + /* disks*/ (vm->def->nnets > 0 ? (4 * vm->def->nnets) : 2) + /* networks */ 1 + /* usb */ @@ -2394,6 +2397,12 @@ if (!((*argv)[++n] = strdup(vcpus))) goto no_memory; + if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_NAME) { + if (!((*argv)[++n] = strdup("-name"))) + goto no_memory; + if (!((*argv)[++n] = strdup(vm->def->name))) + goto no_memory; + } /* * NB, -nographic *MUST* come before any serial, or monitor * or parallel port flags due to QEMU craziness, where it @@ -2472,11 +2481,11 @@ } /* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */ - if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_OPT) { + if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE) { int bootCD = 0, bootFloppy = 0, bootDisk = 0; /* If QEMU supports boot=on for -drive param... */ - if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_BOOT_OPT) { + if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_BOOT) { for (i = 0 ; i < vm->def->os.nBootDevs ; i++) { switch (vm->def->os.bootDevs[i]) { case QEMUD_BOOT_CDROM: diff -r aab96c0c6974 src/qemu_conf.h --- a/src/qemu_conf.h Fri May 09 19:24:37 2008 -0400 +++ b/src/qemu_conf.h Fri May 09 19:30:26 2008 -0400 @@ -249,8 +249,9 @@ QEMUD_CMD_FLAG_KQEMU = (1 << 0), QEMUD_CMD_FLAG_VNC_COLON = (1 << 1), QEMUD_CMD_FLAG_NO_REBOOT = (1 << 2), - QEMUD_CMD_FLAG_DRIVE_OPT = (1 << 3), - QEMUD_CMD_FLAG_DRIVE_BOOT_OPT = (1 << 4), + QEMUD_CMD_FLAG_DRIVE = (1 << 3), + QEMUD_CMD_FLAG_DRIVE_BOOT = (1 << 4), + QEMUD_CMD_FLAG_NAME = (1 << 5), }; diff -r aab96c0c6974 tests/qemuxml2argvdata/qemuxml2argv-minimal.args --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal.args Fri May 09 19:24:37 2008 -0400 +++ b/tests/qemuxml2argvdata/qemuxml2argv-minimal.args Fri May 09 19:30:26 2008 -0400 @@ -1,1 +1,1 @@ -/usr/bin/qemu -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb \ No newline at end of file +/usr/bin/qemu -M pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor pty -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb \ No newline at end of file diff -r aab96c0c6974 tests/qemuxml2argvtest.c --- a/tests/qemuxml2argvtest.c Fri May 09 19:24:37 2008 -0400 +++ b/tests/qemuxml2argvtest.c Fri May 09 19:30:26 2008 -0400 @@ -20,7 +20,7 @@ #define MAX_FILE 4096 -static int testCompareXMLToArgvFiles(const char *xml, const char *cmd, int driveFlag) { +static int testCompareXMLToArgvFiles(const char *xml, const char *cmd, int extraFlags) { char xmlData[MAX_FILE]; char argvData[MAX_FILE]; char *xmlPtr = &(xmlData[0]); @@ -47,10 +47,7 @@ vm.qemuVersion = 0 * 1000 * 100 + (8 * 1000) + 1; vm.qemuCmdFlags = QEMUD_CMD_FLAG_VNC_COLON | QEMUD_CMD_FLAG_NO_REBOOT; - if (driveFlag) { - vm.qemuCmdFlags |= QEMUD_CMD_FLAG_DRIVE_OPT; - vm.qemuCmdFlags |= QEMUD_CMD_FLAG_DRIVE_BOOT_OPT; - } + vm.qemuCmdFlags |= extraFlags; vm.migrateFrom[0] = '\0'; vmdef->vncActivePort = vmdef->vncPort; @@ -100,7 +97,7 @@ struct testInfo { const char *name; - int driveFlag; + int extraFlags; }; static int testCompareXMLToArgvHelper(const void *data) { @@ -111,7 +108,7 @@ abs_srcdir, info->name); snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", abs_srcdir, info->name); - return testCompareXMLToArgvFiles(xml, args, info->driveFlag); + return testCompareXMLToArgvFiles(xml, args, info->extraFlags); } @@ -135,15 +132,15 @@ driver.caps = qemudCapsInit(); -#define DO_TEST(name, driveFlag) \ +#define DO_TEST(name, extraFlags) \ do { \ - struct testInfo info = { name, driveFlag }; \ + struct testInfo info = { name, extraFlags }; \ if (virtTestRun("QEMU XML-2-ARGV " name, \ 1, testCompareXMLToArgvHelper, &info) < 0) \ ret = -1; \ } while (0) - DO_TEST("minimal", 0); + DO_TEST("minimal", QEMUD_CMD_FLAG_NAME); DO_TEST("boot-cdrom", 0); DO_TEST("boot-network", 0); DO_TEST("boot-floppy", 0); @@ -152,8 +149,10 @@ DO_TEST("disk-cdrom", 0); DO_TEST("disk-floppy", 0); DO_TEST("disk-many", 0); - DO_TEST("disk-virtio", 1); - DO_TEST("disk-xenvbd", 1); + DO_TEST("disk-virtio", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DRIVE_BOOT); + DO_TEST("disk-xenvbd", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DRIVE_BOOT); DO_TEST("graphics-vnc", 0); DO_TEST("graphics-sdl", 0); DO_TEST("input-usbmouse", 0); -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, May 13, 2008 at 12:21:02AM +0100, Daniel P. Berrange wrote:
This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore.
Fine by me, +1
@@ -2348,6 +2350,7 @@ len = 1 + /* qemu */ 2 + /* machine type */ disableKQEMU + /* Disable kqemu */ + (vm->qemuCmdFlags & QEMUD_CMD_FLAG_NAME ? 2 : 0) + /* -name XXX */ 2 * vm->def->ndisks + /* disks*/ (vm->def->nnets > 0 ? (4 * vm->def->nnets) : 2) + /* networks */ 1 + /* usb */ @@ -2394,6 +2397,12 @@
I just start to find that arg length computation a bit long and messy. As we add the args maybe it's time to do that a bit more dynamically, no ? Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Thu, May 15, 2008 at 10:36:18AM -0400, Daniel Veillard wrote:
On Tue, May 13, 2008 at 12:21:02AM +0100, Daniel P. Berrange wrote:
This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore.
Fine by me, +1
@@ -2348,6 +2350,7 @@ len = 1 + /* qemu */ 2 + /* machine type */ disableKQEMU + /* Disable kqemu */ + (vm->qemuCmdFlags & QEMUD_CMD_FLAG_NAME ? 2 : 0) + /* -name XXX */ 2 * vm->def->ndisks + /* disks*/ (vm->def->nnets > 0 ? (4 * vm->def->nnets) : 2) + /* networks */ 1 + /* usb */ @@ -2394,6 +2397,12 @@
I just start to find that arg length computation a bit long and messy. As we add the args maybe it's time to do that a bit more dynamically, no ?
Yes it is getting a little messy. Its probably worth dynamically expanding the array as we add each arg. If we define a simple macro to handle the realloc of argv, and the strdup of the actual arg in one go, it should make the code fairly clear. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, May 15, 2008 at 10:36:18AM -0400, Daniel Veillard wrote:
On Tue, May 13, 2008 at 12:21:02AM +0100, Daniel P. Berrange wrote:
This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore.
Fine by me, +1
Thanks, this is applied now. Dan -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Daniel P. Berrange wrote:
This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore.
xenner also has -uuid and -domid switches, passing those too would be great because they will also show up correctly in xenstore then. cheers, Gerd

On Thu, May 15, 2008 at 09:14:12PM +0200, Gerd Hoffmann wrote:
Daniel P. Berrange wrote:
This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore.
xenner also has -uuid and -domid switches, passing those too would be great because they will also show up correctly in xenstore then.
Good idea, I'll make a patch for this too Dan -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, May 15, 2008 at 10:25:24PM +0100, Daniel P. Berrange wrote:
On Thu, May 15, 2008 at 09:14:12PM +0200, Gerd Hoffmann wrote:
Daniel P. Berrange wrote:
This patch makes libvirt pass the -name argumet to QEMU it if it supported by the QEMU binary in question. THis allows QEMU to set the VNC title and allows Xenner to set the Xen guest name in xenstore.
xenner also has -uuid and -domid switches, passing those too would be great because they will also show up correctly in xenstore then.
Good idea, I'll make a patch for this too
So it wouldn't pass these to ordinary qemu? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Tue, May 13, 2008 at 12:21:02AM +0100, Daniel P. Berrange wrote:
- if (strstr(help, "\n-drive")) - *flags |= QEMUD_CMD_FLAG_DRIVE_OPT; [...] + if (strstr(help, "-drive")) + *flags |= QEMUD_CMD_FLAG_DRIVE;
Why this change? I put the \n in front to prevent something like "disk-drive" in a descriptive text making libvirt think that "-drive" was a valid option. -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/

On Thu, May 15, 2008 at 11:20:50PM +0200, Soren Hansen wrote:
On Tue, May 13, 2008 at 12:21:02AM +0100, Daniel P. Berrange wrote:
- if (strstr(help, "\n-drive")) - *flags |= QEMUD_CMD_FLAG_DRIVE_OPT; [...] + if (strstr(help, "-drive")) + *flags |= QEMUD_CMD_FLAG_DRIVE;
Why this change? I put the \n in front to prevent something like "disk-drive" in a descriptive text making libvirt think that "-drive" was a valid option.
Xenner doesn't start its help output right at the start of line, and there isn't any other output there that causes ambiguity in current qemu help output: # qemu-kvm | grep -- -drive -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][index=i] If one happens to be added in the future, that's fine because -drive will be enabled anyway for future releases. Regards, Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, May 15, 2008 at 10:24:35PM +0100, Daniel P. Berrange wrote:
Xenner doesn't start its help output right at the start of line,
Ah. :/
and there isn't any other output there that causes ambiguity in current qemu help output:
# qemu-kvm | grep -- -drive -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][index=i]
If one happens to be added in the future, that's fine because -drive will be enabled anyway for future releases.
I wasn't only worried about future releases, but also previous ones. I didn't want to have to check back through qemu's vcs for instances of "-drive", so I coded defensively instead. :) What versions of QEmu does libvirt actually claim to support? If we only support recent versions, then we're probably fine. -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/
participants (5)
-
Daniel P. Berrange
-
Daniel Veillard
-
Gerd Hoffmann
-
Richard W.M. Jones
-
Soren Hansen