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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;