Re: [Qemu-devel] [libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

Re: [Qemu-devel] [libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]

[adding qemu]

On 04/14/2015 09:58 AM, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander <mkletzan@redhat.com>
> wrote:
> 
>> Is this not exposed in any way in QEMU?  Do we really need to use this
>> (what we're trying to avoid)?
>>
> 
> That works with the following change:
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 768cef1..1b20a7f 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
> 
>  static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
>      { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE },
> +    { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },

Ouch.  qemu commit 0a7cf21 fixes what would have been a regression in
2.3 at exposing "mem-merge" through query-command-line-options, but it
does NOT expose "vmport", which is a per-architecture option rather than
a generic -machine option.  Which means that even though qemu 2.2
(perhaps wrongly) advertised "vmport" for all machines (even when it was
not supported), 2.3 will not advertise it, and we are hoping for a
better solution in 2.4 for properly advertising vmport on an
as-appropriate basis.

Yes, we WANT to use QMP probing,...

>      { "drive", "discard", QEMU_CAPS_DRIVE_DISCARD },
>      { "realtime", "mlock", QEMU_CAPS_MLOCK },
>      { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT },
> @@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>      if (qemuCaps->version >= 1003000)
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
> 
> -    /* vmport option is supported v2.2.0 onwards */
> -    if (qemuCaps->version >= 2002000)
> -        virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);

...and not version comparison, but we'll need something better in QMP
for 2.3 (which is rather late, since we missed 2.3-rc3) if you can't
come up with anything better for learning whether vmport is supported.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

[Martin Kletzander]

[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]

On Tue, Apr 14, 2015 at 10:07:00AM -0600, Eric Blake wrote:
>[adding qemu]
>
>On 04/14/2015 09:58 AM, Marc-André Lureau wrote:
>> Hi
>>
>> On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander <mkletzan@redhat.com>
>> wrote:
>>
>>> Is this not exposed in any way in QEMU?  Do we really need to use this
>>> (what we're trying to avoid)?
>>>
>>
>> That works with the following change:
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 768cef1..1b20a7f 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
>>
>>  static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
>>      { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE },
>> +    { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
>
>Ouch.  qemu commit 0a7cf21 fixes what would have been a regression in
>2.3 at exposing "mem-merge" through query-command-line-options, but it
>does NOT expose "vmport", which is a per-architecture option rather than
>a generic -machine option.  Which means that even though qemu 2.2
>(perhaps wrongly) advertised "vmport" for all machines (even when it was
>not supported), 2.3 will not advertise it, and we are hoping for a
>better solution in 2.4 for properly advertising vmport on an
>as-appropriate basis.
>
>Yes, we WANT to use QMP probing,...
>
>>      { "drive", "discard", QEMU_CAPS_DRIVE_DISCARD },
>>      { "realtime", "mlock", QEMU_CAPS_MLOCK },
>>      { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT },
>> @@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>>      if (qemuCaps->version >= 1003000)
>>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
>>
>> -    /* vmport option is supported v2.2.0 onwards */
>> -    if (qemuCaps->version >= 2002000)
>> -        virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
>
>...and not version comparison, but we'll need something better in QMP
>for 2.3 (which is rather late, since we missed 2.3-rc3) if you can't
>come up with anything better for learning whether vmport is supported.
>

Ouch, I missed that.  But that's something we need for more than just
vmport attribute, but also all other machine-specific ones :(

I still think this might go in, though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

Hi

On Tue, Apr 14, 2015 at 6:07 PM, Eric Blake <eblake@redhat.com> wrote:

> > +    { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
>
> Ouch.  qemu commit 0a7cf21 fixes what would have been a regression in
> 2.3 at exposing "mem-merge" through query-command-line-options, but it
> does NOT expose "vmport", which is a per-architecture option rather than
> a generic -machine option.  Which means that even though qemu 2.2
> (perhaps wrongly) advertised "vmport" for all machines (even when it was
> not supported), 2.3 will not advertise it, and we are hoping for a
> better solution in 2.4 for properly advertising vmport on an
> as-appropriate basis.
>

Thanks Eric for finding out this regression.

Is anybody working on a better solution? I can imagine either a new qmp
machine-list-properties, or perhaps more reasonably a new properties array
in query-machines reply.


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1457 bytes --]

Re: [Qemu-devel] [libvirt] [PATCH 3/5] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

[Peter Maydell]

On 20 April 2015 at 13:39, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Tue, Apr 14, 2015 at 6:07 PM, Eric Blake <eblake@redhat.com> wrote:
>>
>> > +    { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
>>
>> Ouch.  qemu commit 0a7cf21 fixes what would have been a regression in
>> 2.3 at exposing "mem-merge" through query-command-line-options, but it
>> does NOT expose "vmport", which is a per-architecture option rather than
>> a generic -machine option.  Which means that even though qemu 2.2
>> (perhaps wrongly) advertised "vmport" for all machines (even when it was
>> not supported), 2.3 will not advertise it, and we are hoping for a
>> better solution in 2.4 for properly advertising vmport on an
>> as-appropriate basis.
>
>
> Thanks Eric for finding out this regression.
>
> Is anybody working on a better solution? I can imagine either a new qmp
> machine-list-properties, or perhaps more reasonably a new properties array
> in query-machines reply.

Unless this is an absolutely critical "we cannot ship 2.3 without
a fix" bug (in which case somebody should yell now and you really
need a patch on the list within a day or so), you've probably missed
the 2.3 boat at this point.

-- PMM