[libvirt] [PATCH v2 17/17] qemu: assume various QEMU 0.10 features are always available

[John Ferlan]

On 11/10/2015 06:43 AM, Daniel P. Berrange wrote:
> On Tue, Nov 10, 2015 at 06:22:32AM -0500, John Ferlan wrote:
>>
>>
>> On 11/10/2015 05:31 AM, Daniel P. Berrange wrote:
>>> On Mon, Nov 09, 2015 at 06:28:03PM -0500, John Ferlan wrote:
>>>>
>>>>
>>>> On 11/09/2015 11:24 AM, Daniel P. Berrange wrote:
>>>>> The -sdl and -net ...name=XXX arguments were both introduced
>>>>> in QEMU 0.10, so the QEMU driver can assume they are always
>>>>> available.
>>>>>
>>>>
>>>> The -sdl wasn't really removed it seems - although it did me peeking
>>>> into the rabbit hole for a make check failure...
>>>
>>>> After a bit of debugging - qemuParseCommandLine has the following:
>>>>
>>>>        } else if (STRPREFIX(arg, "-hd") ||
>>>>                    STRPREFIX(arg, "-sd") ||
>>>>                    STRPREFIX(arg, "-fd") ||
>>>>                    STREQ(arg, "-cdrom")) {
>>>>             WANT_VALUE();
>>>>
>>>> If I add:
>>>>
>>>>         } else if (STREQ(arg, "-sdl")) {
>>>>             /* Ignore */
>>>>
>>>> Just before that, then things are happy again.
>>>
>>> Rather than ignoring it, I added this:
>>>
>>>             virDomainGraphicsDefPtr sdl;
>>>             if (VIR_ALLOC(sdl) < 0)
>>>                 goto error;
>>>             sdl->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
>>>
>>
>> This seems to duplicate an allocation later :
>>
>> 	    if (!nographics && def->ngraphics == 0) {
> 
> That deals with case where QEMU would previously default to SDL if
> no args are listed. If you specified '-vnc blah -sdl' it would not
> trigger though, so we need to improve that.
> 
>> It's also not FREE'd or appended to def->graphics
> 
> Sigh, so how about this instead, so we just explicitly trigger
> the existing SDL codepath:
> 

Seems fine to me...

ACK

John
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c016d43..792ada3 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -12759,6 +12759,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
>      qemuDomainCmdlineDefPtr cmd = NULL;
>      virDomainDiskDefPtr disk = NULL;
>      const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS");
> +    bool have_sdl = false;
>  
>      if (pidfile)
>          *pidfile = NULL;
> @@ -12982,10 +12983,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
>                  goto error;
>              }
>          } else if (STREQ(arg, "-sdl")) {
> -            virDomainGraphicsDefPtr sdl;
> -            if (VIR_ALLOC(sdl) < 0)
> -                goto error;
> -            sdl->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
> +            have_sdl = true;
>          } else if (STREQ(arg, "-m")) {
>              int mem;
>              WANT_VALUE();
> @@ -13672,7 +13670,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
>          VIR_FREE(capsdata);
>      }
>  
> -    if (!nographics && def->ngraphics == 0) {
> +    if (!nographics && (def->ngraphics == 0 || have_sdl)) {
>          virDomainGraphicsDefPtr sdl;
>          const char *display = qemuFindEnv(progenv, "DISPLAY");
>          const char *xauth = qemuFindEnv(progenv, "XAUTHORITY");
> 
> 
> Regards,
> Daniel
>