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

Daniel P. Berrange berrange at redhat.com
Tue Nov 10 11:43:25 UTC 2015


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:

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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|




More information about the libvir-list mailing list