[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 4/4] qemu: command: Use -name guest= if available




On 05/04/2016 10:56 AM, Cole Robinson wrote:
> -name guest= is the explicit parameter for passing a VM name. Using
> it is required to allow a VM with an '=' in the name
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1276485
> ---
>  src/qemu/qemu_capabilities.c                         | 2 ++
>  src/qemu/qemu_capabilities.h                         | 1 +
>  src/qemu/qemu_command.c                              | 4 ++++
>  tests/qemucapabilitiesdata/caps_2.1.1-1.caps         | 1 +
>  tests/qemucapabilitiesdata/caps_2.4.0-1.caps         | 1 +
>  tests/qemucapabilitiesdata/caps_2.5.0-1.caps         | 1 +
>  tests/qemucapabilitiesdata/caps_2.6.0-1.caps         | 1 +
>  tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 8 ++++----
>  tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml  | 2 +-
>  tests/qemuxml2argvtest.c                             | 2 +-
>  10 files changed, 17 insertions(+), 6 deletions(-)
> 

You're overloading the name-escape test with the "guest=" option which
I'm never quite sure how others feel about. I would think you'd want a
separate test that just adds the "guest=".

I also see this biting us at some point in the future in some test when
'guest=' becomes the default and all the test outputs have to change to
add that.

I'm sure it seems you're chasing a moving target with changes to the
*.caps files... Recent changes in that space will cause more adjustments
here <sigh>.

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 65c3d69..da47759 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -328,6 +328,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "device-tray-moved-event",
>                "nec-usb-xhci-ports",
>                "virtio-scsi-pci.iothread",
> +              "name-guest",
>      );
>  
>  
> @@ -2666,6 +2667,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
>      { "spice", "gl", QEMU_CAPS_SPICE_GL },
>      { "chardev", "logfile", QEMU_CAPS_CHARDEV_LOGFILE },
>      { "name", "debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS },
> +    { "name", "guest", QEMU_CAPS_NAME_GUEST },
>  };
>  
>  static int
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index e7d0a60..9145f2d 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -359,6 +359,7 @@ typedef enum {
>      QEMU_CAPS_DEVICE_TRAY_MOVED, /* DEVICE_TRAY_MOVED event */
>      QEMU_CAPS_NEC_USB_XHCI_PORTS, /* -device nec-usb-xhci.p3 ports setting */
>      QEMU_CAPS_VIRTIO_SCSI_IOTHREAD, /* virtio-scsi-{pci,ccw}.iothread */
> +    QEMU_CAPS_NAME_GUEST, /* -name guest= */
>  
>      QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c2f55b5..570566c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6790,6 +6790,10 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
>  
>      virCommandAddArg(cmd, "-name");
>  
> +    /* The 'guest' option let's us handle a name with '=' embedded in it */
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_GUEST))
> +        virBufferAddLit(&buf, "guest=");
> +

So, any domain that has qemu 2.1 or later will use the "-name
guest=$name..." naming scheme rather than just "-name $name..."
(regardless of whether there's a '=' in $name or not).

I don't necessarily mind using the guest=$name syntax; however, it could
be quite a surprise to anything that searches on/for "-name $name"
rather than "-name guest=$name" in/for the running qemu

John
>      virBufferEscape(&buf, ',', ",", "%s", def->name);
>  
>      if (cfg->setProcessName &&
> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
> index 729000f..29214bb 100644
> --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
> +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
> @@ -164,4 +164,5 @@
>      <flag name='debug-threads'/>
>      <flag name='device-tray-moved-event'/>
>      <flag name='nec-usb-xhci-ports'/>
> +    <flag name='name-guest'/>
>    </qemuCaps>
> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
> index 8009b8f..7b1de29 100644
> --- a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
> +++ b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
> @@ -177,4 +177,5 @@
>      <flag name='device-tray-moved-event'/>
>      <flag name='nec-usb-xhci-ports'/>
>      <flag name='virtio-scsi-pci.iothread'/>
> +    <flag name='name-guest'/>
>    </qemuCaps>
> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
> index e139455..19192da 100644
> --- a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
> +++ b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
> @@ -178,4 +178,5 @@
>      <flag name='device-tray-moved-event'/>
>      <flag name='nec-usb-xhci-ports'/>
>      <flag name='virtio-scsi-pci.iothread'/>
> +    <flag name='name-guest'/>
>    </qemuCaps>
> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
> index ec5bfcc..59f637f 100644
> --- a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
> +++ b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps
> @@ -183,4 +183,5 @@
>      <flag name='device-tray-moved-event'/>
>      <flag name='nec-usb-xhci-ports'/>
>      <flag name='virtio-scsi-pci.iothread'/>
> +    <flag name='name-guest'/>
>    </qemuCaps>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
> index 772d94f..7ecdca5 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
> @@ -5,17 +5,17 @@ USER=test \
>  LOGNAME=test \
>  QEMU_AUDIO_DRV=none \
>  /usr/bin/qemu \
> --name foo,,bar,debug-threads=on \
> +-name guest=foo=1,,bar=2,debug-threads=on \
>  -S \
> --object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo,,\
> -bar/master-key.aes \
> +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo=1,,\
> +bar=2/master-key.aes \
>  -M pc \
>  -m 214 \
>  -smp 1 \
>  -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
>  -nographic \
>  -nodefaults \
> --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,,bar/monitor.sock,\
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo=1,,bar=2/monitor.sock,\
>  server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
> index 3a8c3cd..0057062 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
> @@ -1,5 +1,5 @@
>  <domain type='qemu'>
> -  <name>foo,bar</name>
> +  <name>foo=1,bar=2</name>
>    <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>    <memory unit='KiB'>219100</memory>
>    <currentMemory unit='KiB'>219100</currentMemory>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index d8834cb..d7b06cf 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1894,7 +1894,7 @@ mymain(void)
>                                NONE);
>  
>      DO_TEST("name-escape", QEMU_CAPS_NAME_DEBUG_THREADS,
> -            QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_CHARDEV);
> +            QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_CHARDEV, QEMU_CAPS_NAME_GUEST);
>      DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
>  
>      DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]