[libvirt] [REPOSTv2 PATCH v3 4/6] qemu: Add VM Generation ID to qemu command line

John Ferlan posted 6 patches 6 years, 12 months ago
[libvirt] [REPOSTv2 PATCH v3 4/6] qemu: Add VM Generation ID to qemu command line
Posted by John Ferlan 6 years, 12 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1149445

If the domain requests usage of the genid functionality,
then add the QEMU '-device vmgenid' to the command line
providing either the supplied or generated GUID value.

Add tests for both a generated and supplied GUID value.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/qemu/qemu_command.c                            | 24 +++++++++++++++++
 .../qemuxml2argvdata/genid-auto.x86_64-latest.args | 30 ++++++++++++++++++++++
 tests/qemuxml2argvdata/genid.x86_64-latest.args    | 30 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  4 +++
 4 files changed, 88 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/genid-auto.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/genid.x86_64-latest.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c4237339bf..78bd685008 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6002,6 +6002,27 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
 }
 
 
+static int
+qemuBuildVMGenIDCommandLine(virCommandPtr cmd,
+                            const virDomainDef *def)
+{
+    virBuffer opts = VIR_BUFFER_INITIALIZER;
+    char guid[VIR_UUID_STRING_BUFLEN];
+
+    if (!def->genidRequested)
+        return 0;
+
+    virUUIDFormat(def->genid, guid);
+    virBufferAsprintf(&opts, "vmgenid,guid=%s,id=vmgenid0", guid);
+
+    virCommandAddArg(cmd, "-device");
+    virCommandAddArgBuffer(cmd, &opts);
+
+    virBufferFreeAndReset(&opts);
+    return 0;
+}
+
+
 static int
 qemuBuildSgaCommandLine(virCommandPtr cmd,
                         const virDomainDef *def,
@@ -9995,6 +10016,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
     if (qemuBuildSmbiosCommandLine(cmd, driver, def) < 0)
         goto error;
 
+    if (qemuBuildVMGenIDCommandLine(cmd, def) < 0)
+        goto error;
+
     /*
      * NB, -nographic *MUST* come before any serial, or monitor
      * or parallel port flags due to QEMU craziness, where it
diff --git a/tests/qemuxml2argvdata/genid-auto.x86_64-latest.args b/tests/qemuxml2argvdata/genid-auto.x86_64-latest.args
new file mode 100644
index 0000000000..ce163020b9
--- /dev/null
+++ b/tests/qemuxml2argvdata/genid-auto.x86_64-latest.args
@@ -0,0 +1,30 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-device vmgenid,guid=00010203-0405-4607-8809-0a0b0c0d0e0f,id=vmgenid0 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/genid.x86_64-latest.args b/tests/qemuxml2argvdata/genid.x86_64-latest.args
new file mode 100644
index 0000000000..54e00f4bdb
--- /dev/null
+++ b/tests/qemuxml2argvdata/genid.x86_64-latest.args
@@ -0,0 +1,30 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-device vmgenid,guid=e9392370-2917-565e-692b-d057f46512d6,id=vmgenid0 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 07e5ba1d13..025fe407e0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -787,6 +787,10 @@ mymain(void)
             QEMU_CAPS_SECCOMP_BLACKLIST);
     DO_TEST_PARSE_ERROR("minimal-no-memory", NONE);
     DO_TEST("minimal-msg-timestamp", QEMU_CAPS_MSG_TIMESTAMP);
+
+    DO_TEST_CAPS_LATEST("genid");
+    DO_TEST_CAPS_LATEST("genid-auto");
+
     DO_TEST("machine-aliases1", NONE);
     DO_TEST("machine-aliases2", QEMU_CAPS_KVM);
     DO_TEST("machine-core-on", NONE);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOSTv2 PATCH v3 4/6] qemu: Add VM Generation ID to qemu command line
Posted by Michal Privoznik 6 years, 11 months ago
On 05/17/2018 02:42 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1149445
> 
> If the domain requests usage of the genid functionality,
> then add the QEMU '-device vmgenid' to the command line
> providing either the supplied or generated GUID value.
> 
> Add tests for both a generated and supplied GUID value.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/qemu/qemu_command.c                            | 24 +++++++++++++++++
>  .../qemuxml2argvdata/genid-auto.x86_64-latest.args | 30 ++++++++++++++++++++++
>  tests/qemuxml2argvdata/genid.x86_64-latest.args    | 30 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  4 +++
>  4 files changed, 88 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/genid-auto.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/genid.x86_64-latest.args
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c4237339bf..78bd685008 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6002,6 +6002,27 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
>  }
>  
>  
> +static int
> +qemuBuildVMGenIDCommandLine(virCommandPtr cmd,
> +                            const virDomainDef *def)
> +{
> +    virBuffer opts = VIR_BUFFER_INITIALIZER;
> +    char guid[VIR_UUID_STRING_BUFLEN];
> +
> +    if (!def->genidRequested)
> +        return 0;
> +
> +    virUUIDFormat(def->genid, guid);
> +    virBufferAsprintf(&opts, "vmgenid,guid=%s,id=vmgenid0", guid);
> +
> +    virCommandAddArg(cmd, "-device");
> +    virCommandAddArgBuffer(cmd, &opts);
> +
> +    virBufferFreeAndReset(&opts);
> +    return 0;

As mentioned in previous patch this function must check for
DEVICE_VMGENID capability. Otherwise we might put -device vmgenid on
command line for qemu that doesn't support it.

> +}

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOSTv2 PATCH v3 4/6] qemu: Add VM Generation ID to qemu command line
Posted by John Ferlan 6 years, 11 months ago

On 05/25/2018 04:24 AM, Michal Privoznik wrote:
> On 05/17/2018 02:42 PM, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1149445
>>
>> If the domain requests usage of the genid functionality,
>> then add the QEMU '-device vmgenid' to the command line
>> providing either the supplied or generated GUID value.
>>
>> Add tests for both a generated and supplied GUID value.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>  src/qemu/qemu_command.c                            | 24 +++++++++++++++++
>>  .../qemuxml2argvdata/genid-auto.x86_64-latest.args | 30 ++++++++++++++++++++++
>>  tests/qemuxml2argvdata/genid.x86_64-latest.args    | 30 ++++++++++++++++++++++
>>  tests/qemuxml2argvtest.c                           |  4 +++
>>  4 files changed, 88 insertions(+)
>>  create mode 100644 tests/qemuxml2argvdata/genid-auto.x86_64-latest.args
>>  create mode 100644 tests/qemuxml2argvdata/genid.x86_64-latest.args
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index c4237339bf..78bd685008 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -6002,6 +6002,27 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
>>  }
>>  
>>  
>> +static int
>> +qemuBuildVMGenIDCommandLine(virCommandPtr cmd,
>> +                            const virDomainDef *def)
>> +{
>> +    virBuffer opts = VIR_BUFFER_INITIALIZER;
>> +    char guid[VIR_UUID_STRING_BUFLEN];
>> +
>> +    if (!def->genidRequested)
>> +        return 0;
>> +
>> +    virUUIDFormat(def->genid, guid);
>> +    virBufferAsprintf(&opts, "vmgenid,guid=%s,id=vmgenid0", guid);
>> +
>> +    virCommandAddArg(cmd, "-device");
>> +    virCommandAddArgBuffer(cmd, &opts);
>> +
>> +    virBufferFreeAndReset(&opts);
>> +    return 0;
> 
> As mentioned in previous patch this function must check for
> DEVICE_VMGENID capability. Otherwise we might put -device vmgenid on
> command line for qemu that doesn't support it.
> 

Which path would do that?  I believe I'm making "interesting" use of the
DO_TEST_CAPS_LATEST actually.

There are two ways to call qemuBuildCommandLine - one via the PretendCmd
logic and one via the Launch logic.

In the Launch logic prior to calling qemuBuildCommandLine, the
capability is checked *and* the "need" for generating a new GUID is
determined. It was a "design decision" to not check/change it earlier.
Is there somewhere else you believe that code should go? IDC, I can move
it, but understand the risk of doing so.

Now that we have DO_TEST_CAPS_LATEST there's no need to pass an explicit
flag for the PretendCmd line logic, so we get it for free for the
test/fake command line.

John

>> +}
> 
> Michal
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOSTv2 PATCH v3 4/6] qemu: Add VM Generation ID to qemu command line
Posted by Michal Privoznik 6 years, 11 months ago
On 05/25/2018 12:58 PM, John Ferlan wrote:
> 
> 
> On 05/25/2018 04:24 AM, Michal Privoznik wrote:
>> On 05/17/2018 02:42 PM, John Ferlan wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1149445
>>>
>>> If the domain requests usage of the genid functionality,
>>> then add the QEMU '-device vmgenid' to the command line
>>> providing either the supplied or generated GUID value.
>>>
>>> Add tests for both a generated and supplied GUID value.
>>>
>>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>>> ---
>>>  src/qemu/qemu_command.c                            | 24 +++++++++++++++++
>>>  .../qemuxml2argvdata/genid-auto.x86_64-latest.args | 30 ++++++++++++++++++++++
>>>  tests/qemuxml2argvdata/genid.x86_64-latest.args    | 30 ++++++++++++++++++++++
>>>  tests/qemuxml2argvtest.c                           |  4 +++
>>>  4 files changed, 88 insertions(+)
>>>  create mode 100644 tests/qemuxml2argvdata/genid-auto.x86_64-latest.args
>>>  create mode 100644 tests/qemuxml2argvdata/genid.x86_64-latest.args
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index c4237339bf..78bd685008 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -6002,6 +6002,27 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
>>>  }
>>>  
>>>  
>>> +static int
>>> +qemuBuildVMGenIDCommandLine(virCommandPtr cmd,
>>> +                            const virDomainDef *def)
>>> +{
>>> +    virBuffer opts = VIR_BUFFER_INITIALIZER;
>>> +    char guid[VIR_UUID_STRING_BUFLEN];
>>> +
>>> +    if (!def->genidRequested)
>>> +        return 0;
>>> +
>>> +    virUUIDFormat(def->genid, guid);
>>> +    virBufferAsprintf(&opts, "vmgenid,guid=%s,id=vmgenid0", guid);
>>> +
>>> +    virCommandAddArg(cmd, "-device");
>>> +    virCommandAddArgBuffer(cmd, &opts);
>>> +
>>> +    virBufferFreeAndReset(&opts);
>>> +    return 0;
>>
>> As mentioned in previous patch this function must check for
>> DEVICE_VMGENID capability. Otherwise we might put -device vmgenid on
>> command line for qemu that doesn't support it.
>>
> 
> Which path would do that?  I believe I'm making "interesting" use of the
> DO_TEST_CAPS_LATEST actually.
> 
> There are two ways to call qemuBuildCommandLine - one via the PretendCmd
> logic and one via the Launch logic.
> 
> In the Launch logic prior to calling qemuBuildCommandLine, the
> capability is checked *and* the "need" for generating a new GUID is
> determined. It was a "design decision" to not check/change it earlier.
> Is there somewhere else you believe that code should go? IDC, I can move
> it, but understand the risk of doing so.
> 
> Now that we have DO_TEST_CAPS_LATEST there's no need to pass an explicit
> flag for the PretendCmd line logic, so we get it for free for the
> test/fake command line.

Well, qemuConnectDomainXMLToNative() for instance. So I suggest leaving
the capability check in qemuProcessGenID() and having it repeated here.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list