[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] PATCH: Explicit detection of KVM version



On Thu, Sep 20, 2007 at 05:07:30PM +0100, Daniel P. Berrange wrote:
> On Thu, Sep 20, 2007 at 10:45:03AM +0200, Gerd Hoffmann wrote:
> > Daniel P. Berrange wrote:
> > > Before starting any guest, the QEMU driver needs to figure out what version
> > > of QEMU is in use & thus determine whether it supports particular command 
> > > line flags. We currently do that just by calling /usr/bin/qemu, since all
> > > the various qemu-system-XXX binaries share the same syntax. The only problem
> > > is that qemu-kvm does not neccessarily match the version of qemu installed.
> > > So we detect QEMU version 0.8.2, but KVM is 0.9.0 based. The result is that
> > > we pass the wrong style VNC argument to KVM & it fails to start. The second
> > > problem is that even if you only ever want to run KVM guests, you still have
> > > to have KVM itself installed.
> > 
> > We have the same problem if the domain.xml has
> > "<emulator>/path/to/qemu-foo</emulator>" specified, which I think isn't
> > addressed by this patch ...
> 
> Nope it is not addressed. I think maybe instead of detecting once for QEMU
> and KVM for the QEMU driver as  whole, I'll move the flags into the per-VM
> data structure. This will have extra overhead, since for each distinct VM
> we'll have todo a test launch of QEMU, but VM creation is not a performance
> bottleneck so I don't think its an issue.

Here is an updated patch which makes the version/flags detection per-VM
in the qemu driver. We still cache the data, but invalidate it whenever
the VM's config is changed.

Tested with KVM when a differeing QEMU version is installed, and when no QEMU
is installed at all & both scenarios now work correctly.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
Index: src/qemu_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.13
diff -u -p -r1.13 qemu_conf.c
--- src/qemu_conf.c	13 Sep 2007 22:06:54 -0000	1.13
+++ src/qemu_conf.c	20 Sep 2007 16:54:38 -0000
@@ -287,7 +287,6 @@ static const char *qemudDefaultBinaryFor
 
 /* Find the fully qualified path to the binary for an architecture */
 static char *qemudLocateBinaryForArch(virConnectPtr conn,
-                                      struct qemud_driver *driver ATTRIBUTE_UNUSED,
                                       int virtType, const char *arch) {
     const char *name;
     char *path;
@@ -408,14 +407,15 @@ static int qemudExtractVersionInfo(const
 }
 
 int qemudExtractVersion(virConnectPtr conn,
-                        struct qemud_driver *driver) {
+                        struct qemud_driver *driver ATTRIBUTE_UNUSED) {
     char *binary = NULL;
     struct stat sb;
+    int ignored;
 
     if (driver->qemuVersion > 0)
         return 0;
 
-    if (!(binary = qemudLocateBinaryForArch(conn, driver, QEMUD_VIRT_QEMU, "i686")))
+    if (!(binary = qemudLocateBinaryForArch(conn, QEMUD_VIRT_QEMU, "i686")))
         return -1;
 
     if (stat(binary, &sb) < 0) {
@@ -426,7 +426,7 @@ int qemudExtractVersion(virConnectPtr co
         return -1;
     }
 
-    if (qemudExtractVersionInfo(binary, &driver->qemuVersion, &driver->qemuCmdFlags) < 0) {
+    if (qemudExtractVersionInfo(binary, &driver->qemuVersion, &ignored) < 0) {
         free(binary);
         return -1;
     }
@@ -1199,7 +1199,7 @@ static struct qemud_vm_def *qemudParseXM
     obj = xmlXPathEval(BAD_CAST "string(/domain/devices/emulator[1])", ctxt);
     if ((obj == NULL) || (obj->type != XPATH_STRING) ||
         (obj->stringval == NULL) || (obj->stringval[0] == 0)) {
-        char *tmp = qemudLocateBinaryForArch(conn, driver, def->virtType, def->os.arch);
+        char *tmp = qemudLocateBinaryForArch(conn, def->virtType, def->os.arch);
         if (!tmp) {
             goto error;
         }
@@ -1466,8 +1466,23 @@ int qemudBuildCommandLine(virConnectPtr 
     struct utsname ut;
     int disableKQEMU = 0;
 
-    if (qemudExtractVersion(conn, driver) < 0)
+    /* Make sure the binary we are about to try exec'ing exists.
+     * Technically we could catch the exec() failure, but that's
+     * in a sub-process so its hard to feed back a useful error
+     */
+    if (stat(vm->def->os.binary, &sb) < 0) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "Cannot find QEMU binary %s: %s", vm->def->os.binary,
+                         strerror(errno));
         return -1;
+    }
+
+    if (vm->qemuVersion == 0) {
+        if (qemudExtractVersionInfo(vm->def->os.binary,
+                                    &(vm->qemuVersion),
+                                    &(vm->qemuCmdFlags)) < 0)
+            return -1;
+    }
 
     uname(&ut);
 
@@ -1483,22 +1498,11 @@ int qemudBuildCommandLine(virConnectPtr 
      * 2. Guest is 'qemu'
      * 3. The qemu binary has the -no-kqemu flag
      */
-    if ((driver->qemuCmdFlags & QEMUD_CMD_FLAG_KQEMU) &&
+    if ((vm->qemuCmdFlags & QEMUD_CMD_FLAG_KQEMU) &&
         !strcmp(ut.machine, vm->def->os.arch) &&
         vm->def->virtType == QEMUD_VIRT_QEMU)
         disableKQEMU = 1;
 
-    /* Make sure the binary we are about to try exec'ing exists.
-     * Technically we could catch the exec() failure, but that's
-     * in a sub-process so its hard to feed back a useful error
-     */
-    if (stat(vm->def->os.binary, &sb) < 0) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                         "Cannot find QEMU binary %s: %s", vm->def->os.binary,
-                         strerror(errno));
-        return -1;
-    }
-
     len = 1 + /* qemu */
         2 + /* machine type */
         disableKQEMU + /* Disable kqemu */
@@ -1511,7 +1515,7 @@ int qemudBuildCommandLine(virConnectPtr 
         2 + /* boot device */
         2 + /* monitor */
         (vm->def->localtime ? 1 : 0) + /* localtime */
-        (driver->qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT &&
+        (vm->qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT &&
          vm->def->noReboot ? 1 : 0) + /* no-reboot */
         (vm->def->features & QEMUD_FEATURE_ACPI ? 0 : 1) + /* acpi */
         (vm->def->os.kernel[0] ? 2 : 0) + /* kernel */
@@ -1567,7 +1571,7 @@ int qemudBuildCommandLine(virConnectPtr 
             goto no_memory;
     }
 
-    if (driver->qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT &&
+    if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT &&
         vm->def->noReboot) {
         if (!((*argv)[++n] = strdup("-no-reboot")))
             goto no_memory;
@@ -1748,7 +1752,7 @@ int qemudBuildCommandLine(virConnectPtr 
     if (vm->def->graphicsType == QEMUD_GRAPHICS_VNC) {
         char vncdisplay[BR_INET_ADDR_MAXLEN+20];
         int ret;
-        if (driver->qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON)
+        if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON)
             ret = snprintf(vncdisplay, sizeof(vncdisplay), "%s:%d",
                            vm->def->vncListen,
                            vm->def->vncActivePort - 5900);
@@ -1885,7 +1889,9 @@ qemudAssignVMDef(virConnectPtr conn,
                 qemudFreeVMDef(vm->newDef);
             vm->newDef = def;
         }
-
+        /* Reset version, because the emulator path might have changed */
+        vm->qemuVersion = 0;
+        vm->qemuCmdFlags = 0;
         return vm;
     }
 
Index: src/qemu_conf.h
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_conf.h,v
retrieving revision 1.8
diff -u -p -r1.8 qemu_conf.h
--- src/qemu_conf.h	14 Aug 2007 01:28:47 -0000	1.8
+++ src/qemu_conf.h	20 Sep 2007 16:54:38 -0000
@@ -211,6 +211,9 @@ struct qemud_vm {
     int *tapfds;
     int ntapfds;
 
+    int qemuVersion;
+    int qemuCmdFlags; /* values from enum qemud_cmd_flags */
+
     char configFile[PATH_MAX];
     char autostartLink[PATH_MAX];
     char migrateFrom[PATH_MAX];
@@ -272,7 +275,6 @@ struct qemud_network {
 /* Main driver state */
 struct qemud_driver {
     int qemuVersion;
-    int qemuCmdFlags; /* values from enum qemud_cmd_flags */
     int nactivevms;
     int ninactivevms;
     struct qemud_vm *vms;
Index: tests/qemuxml2argvtest.c
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvtest.c,v
retrieving revision 1.4
diff -u -p -r1.4 qemuxml2argvtest.c
--- tests/qemuxml2argvtest.c	13 Sep 2007 22:06:54 -0000	1.4
+++ tests/qemuxml2argvtest.c	20 Sep 2007 16:54:38 -0000
@@ -37,6 +37,10 @@ static int testCompareXMLToArgvFiles(con
     vm.def = vmdef;
     vm.pid = -1;
     vm.id = -1;
+    vm.qemuVersion = 0 * 1000 * 100 + (8 * 1000) + 1;
+    vm.qemuCmdFlags = QEMUD_CMD_FLAG_VNC_COLON |
+        QEMUD_CMD_FLAG_NO_REBOOT;
+
     vmdef->vncActivePort = vmdef->vncPort;
 
     if (qemudBuildCommandLine(NULL, &driver, &vm, &argv) < 0)
@@ -107,10 +111,6 @@ main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    driver.qemuVersion = 0 * 1000 * 100 + (8 * 1000) + 1;
-    driver.qemuCmdFlags = QEMUD_CMD_FLAG_VNC_COLON | 
-        QEMUD_CMD_FLAG_NO_REBOOT;
-
     if (virtTestRun("QEMU XML-2-ARGV minimal",
                     1, testCompareXMLToArgvHelper, "minimal") < 0)
         ret = -1;
Index: tests/qemuxml2xmltest.c
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2xmltest.c,v
retrieving revision 1.4
diff -u -p -r1.4 qemuxml2xmltest.c
--- tests/qemuxml2xmltest.c	13 Sep 2007 22:06:54 -0000	1.4
+++ tests/qemuxml2xmltest.c	20 Sep 2007 16:54:38 -0000
@@ -31,6 +31,10 @@ static int testCompareXMLToXMLFiles(cons
     vm.def = vmdef;
     vm.pid = -1;
     vm.id = -1;
+    vm.qemuVersion = 0 * 1000 * 100 + (8 * 1000) + 1;
+    vm.qemuCmdFlags = QEMUD_CMD_FLAG_VNC_COLON |
+        QEMUD_CMD_FLAG_NO_REBOOT;
+
     vmdef->vncActivePort = vmdef->vncPort;
 
     if (!(actual = qemudGenerateXML(NULL, &driver, &vm, vmdef, 0)))
@@ -72,10 +76,6 @@ main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    driver.qemuVersion = 0 * 1000 * 100 + (8 * 1000) + 1;
-    driver.qemuCmdFlags = QEMUD_CMD_FLAG_VNC_COLON | 
-        QEMUD_CMD_FLAG_NO_REBOOT;
-
     if (virtTestRun("QEMU XML-2-ARGV minimal",
                     1, testCompareXMLToXMLHelper, "minimal") < 0)
         ret = -1;

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]