[libvirt] [PATCH 1/8] qemu: parse: drop redundant video config

[Cole Robinson]

On 07/18/2017 06:20 PM, John Ferlan wrote:
> 
> 
> On 06/28/2017 02:49 PM, Cole Robinson wrote:
>> The ram/vram = 0 bits aren't needed, and PostParse will fill in the
>> needed QXL default
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>>  src/qemu/qemu_command.c       |  4 ----
>>  src/qemu/qemu_parse_command.c | 11 +----------
>>  2 files changed, 1 insertion(+), 14 deletions(-)
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index c53ab97b9..ebf9c63bc 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -7264,10 +7264,6 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
>>          }
>>          break;
>>  
>> -    case VIR_DOMAIN_VIRT_XEN:
>> -        /* XXX better check for xenner */
>> -        break;
>> -
>>      default:
>>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>                         _("the QEMU binary does not support %s"),
>> diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
>> index af9063c02..60c81f0ca 100644
>> --- a/src/qemu/qemu_parse_command.c
>> +++ b/src/qemu/qemu_parse_command.c
>> @@ -2608,16 +2608,7 @@ qemuParseCommandLine(virCapsPtr caps,
>>          virDomainVideoDefPtr vid;
>>          if (VIR_ALLOC(vid) < 0)
>>              goto error;
>> -        if (def->virtType == VIR_DOMAIN_VIRT_XEN)
>> -            vid->type = VIR_DOMAIN_VIDEO_TYPE_XEN;
> 
> Did your follow-up comment mean to imply this would be done separately
> too or just the qemu_command.c change?
> 
> I guess I see video default'ing to VIR_DOMAIN_VIDEO_TYPE_CIRRUS and
> changing based on the presence of various qualifiers, but this seems to
> force a specific type based on virtType for virtType == VIRT_XEN that
> would seemingly be lost. What is actually on the command line I didn't
> chase down...
> 
> Still the qemu_parse_command code isn't exactly highly used and I think
> as long as it can be explained why there's not need to special case
> VIRT_XEN any more perhaps in the commit message, then you've got my:
> 
> Reviewed-by: John Ferlan <jferlan at redhat.com>
> 

Thanks for the review and sorry for the delayed response. I split out the xen
changes into their own commit with this message:

    qemu: Remove remnants of xenner support

    Both of these are dead code: qemu_command.c explicitly rejects
    VIRT_XEN earlier in the call chain, and qemu_parse_command.c
    will never set VIRT_XEN anymore

I pushed it along with patch 1, 2, and 4, since patch 3 is independent and has
some comments

Thanks,
Cole