[libvirt] avoid double free

2008-05-16 Thread Jim Meyering
I noticed a new test failure on rawhide,
ran valgrind ./qemuxml2xmltest and got this:

==14847== Invalid free() / delete / delete[]
==14847==at 0x4A0609F: free (vg_replace_malloc.c:323)
==14847==by 0x409DF8: qemudParseXML (qemu_conf.c:2149)
==14847==by 0x40CBE0: qemudParseVMDef (qemu_conf.c:2982)
==14847==by 0x4021CD: testCompareXMLToXMLFiles (qemuxml2xmltest.c:35)
==14847==by 0x4022EA: testCompareXMLToXMLHelper (qemuxml2xmltest.c:68)
==14847==by 0x40291B: virtTestRun (testutils.c:79)
==14847==by 0x402436: main (qemuxml2xmltest.c:100)
==14847==  Address 0x4cd3d68 is 0 bytes inside a block of size 72 free'd
==14847==at 0x4A0609F: free (vg_replace_malloc.c:323)
==14847==by 0x408731: qemudParseXML (qemu_conf.c:1738)
==14847==by 0x40CBE0: qemudParseVMDef (qemu_conf.c:2982)
==14847==by 0x4021CD: testCompareXMLToXMLFiles (qemuxml2xmltest.c:35)
==14847==by 0x4022EA: testCompareXMLToXMLHelper (qemuxml2xmltest.c:68)
==14847==by 0x40291B: virtTestRun (testutils.c:79)
==14847==by 0x402436: main (qemuxml2xmltest.c:100)

Here's the fix:

From 777e199f2d680ec302b7604e030a41da2c62cb49 Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Fri, 16 May 2008 08:18:31 +0200
Subject: [PATCH] avoid a double-free bug

* src/qemu_conf.c (qemudParseXML): Ensure that obj is either
NULL or a valid malloc'd pointer before we might goto error
where it is freed.
---
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 458f5df..1a7ab46 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1736,6 +1736,7 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr 
conn,
 } else {
 strcpy(def-os.type, (const char *)obj-stringval);
 xmlXPathFreeObject(obj);
+obj = NULL;
 }

 if (!virCapabilitiesSupportsGuestOSType(driver-caps, def-os.type)) {
--
1.5.5.1.249.g26848

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] new qemu tests require xen support

2008-05-16 Thread Jim Meyering
Hi Dan,

On a system with stock rawhide kernel (not the xen one), I get two
new test failures because virCapabilitiesSupportsGuestOSType(caps, xen)
returns 0.

$ ./qemuxml2xmltest
 1) QEMU XML-2-XML minimal... OK
 2) QEMU XML-2-XML boot-cdrom ... OK
 3) QEMU XML-2-XML boot-network   ... OK
 4) QEMU XML-2-XML boot-floppy... OK
 5) QEMU XML-2-XML bootloader ... 
libvir: QEMU error : unknown OS type xen
FAILED
 6) QEMU XML-2-XML clock-utc  ... OK
 7) QEMU XML-2-XML clock-localtime... OK
 8) QEMU XML-2-XML disk-cdrom ... OK
 9) QEMU XML-2-XML disk-floppy... OK
10) QEMU XML-2-XML disk-many  ... OK
11) QEMU XML-2-XML disk-xenvbd... OK
12) QEMU XML-2-XML graphics-vnc   ... OK
13) QEMU XML-2-XML graphics-sdl   ... OK
14) QEMU XML-2-XML input-usbmouse ... OK
15) QEMU XML-2-XML input-usbtablet... OK
16) QEMU XML-2-XML input-xen  ... 
libvir: QEMU error : unknown OS type xen
FAILED
17) QEMU XML-2-XML misc-acpi  ... OK
18) QEMU XML-2-XML misc-no-reboot ... OK
19) QEMU XML-2-XML net-user   ... OK
20) QEMU XML-2-XML net-virtio ... OK
21) QEMU XML-2-XML serial-vc  ... OK
22) QEMU XML-2-XML serial-pty ... OK
23) QEMU XML-2-XML serial-dev ... OK
24) QEMU XML-2-XML serial-file... OK
25) QEMU XML-2-XML serial-unix... OK
26) QEMU XML-2-XML serial-tcp ... OK
27) QEMU XML-2-XML serial-udp ... OK
28) QEMU XML-2-XML serial-tcp-telnet  ... OK
29) QEMU XML-2-XML serial-many... OK
30) QEMU XML-2-XML parallel-tcp   ... OK
31) QEMU XML-2-XML console-compat ... OK
[Exit 1]

qemuxml2argvtest fails the same way.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] start using c-ctype functions

2008-05-16 Thread Daniel Veillard
On Thu, May 15, 2008 at 05:56:57PM +0200, Jim Meyering wrote:
 
 From db0ed598dfc045f3937a3629a432d2d703449f50 Mon Sep 17 00:00:00 2001
 From: Jim Meyering [EMAIL PROTECTED]
 Date: Wed, 14 May 2008 23:06:15 +0200
 Subject: [PATCH] start using c-ctype functions
 
 Up to now, we've been avoiding ctype functions like isspace, isdigit, etc.
 Now that we have the c-ctype functions, we can start using them to make
 the code more readable with changes like these:

  okay in principle, as stated previously


 -SKIP_SPACES;
 +SKIP_BLANKS;
[...]
 -SKIP_BLANKS;
 +SKIP_BLANKS_AND_EOL;

  i was surprized by the macro renaming initially

 +/* Convert C from hexadecimal character to integer.  */
 +static int
 +hextobin (unsigned char c)
 +{
 +  switch (c)
 +{
 +default: return c - '0';
 +case 'a': case 'A': return 10;
 +case 'b': case 'B': return 11;
 +case 'c': case 'C': return 12;
 +case 'd': case 'D': return 13;
 +case 'e': case 'E': return 14;
 +case 'f': case 'F': return 15;
 +}
 +}

  putting the default first feels strange to me, i guess it's the
first time I see a switch that way.


  Patch looks fine to me :-)

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] avoid double free

2008-05-16 Thread Daniel Veillard
On Fri, May 16, 2008 at 08:21:07AM +0200, Jim Meyering wrote:
 I noticed a new test failure on rawhide,
 ran valgrind ./qemuxml2xmltest and got this:

  Sure, +1, thanks !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] start using c-ctype functions

2008-05-16 Thread Jim Meyering
Daniel Veillard [EMAIL PROTECTED] wrote:
 On Thu, May 15, 2008 at 05:56:57PM +0200, Jim Meyering wrote:
 From db0ed598dfc045f3937a3629a432d2d703449f50 Mon Sep 17 00:00:00 2001
 From: Jim Meyering [EMAIL PROTECTED]
 Date: Wed, 14 May 2008 23:06:15 +0200
 Subject: [PATCH] start using c-ctype functions

 Up to now, we've been avoiding ctype functions like isspace, isdigit, etc.
 Now that we have the c-ctype functions, we can start using them to make
 the code more readable with changes like these:

   okay in principle, as stated previously


 -SKIP_SPACES;
 +SKIP_BLANKS;
 [...]
 -SKIP_BLANKS;
 +SKIP_BLANKS_AND_EOL;

   i was surprized by the macro renaming initially

 +/* Convert C from hexadecimal character to integer.  */
 +static int
 +hextobin (unsigned char c)
 +{
 +  switch (c)
 +{
 +default: return c - '0';
 +case 'a': case 'A': return 10;
 +case 'b': case 'B': return 11;
 +case 'c': case 'C': return 12;
 +case 'd': case 'D': return 13;
 +case 'e': case 'E': return 14;
 +case 'f': case 'F': return 15;
 +}
 +}

   putting the default first feels strange to me, i guess it's the
 first time I see a switch that way.

   Patch looks fine to me :-)

Thanks for the review.
I've just committed that and the double-free fix.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] start using c-ctype functions

2008-05-16 Thread Jim Meyering
Daniel Veillard [EMAIL PROTECTED] wrote:
 +/* Convert C from hexadecimal character to integer.  */
 +static int
 +hextobin (unsigned char c)
 +{
 +  switch (c)
 +{
 +default: return c - '0';
 +case 'a': case 'A': return 10;
 +case 'b': case 'B': return 11;
 +case 'c': case 'C': return 12;
 +case 'd': case 'D': return 13;
 +case 'e': case 'E': return 14;
 +case 'f': case 'F': return 15;
 +}
 +}

   putting the default first feels strange to me, i guess it's the
 first time I see a switch that way.

BTW, I too found that odd.
It's copied verbatim from coreutils/src/echo.c ;-)

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: new qemu tests require xen support

2008-05-16 Thread Daniel P. Berrange
On Fri, May 16, 2008 at 09:06:41AM +0200, Jim Meyering wrote:
 Hi Dan,
 
 On a system with stock rawhide kernel (not the xen one), I get two
 new test failures because virCapabilitiesSupportsGuestOSType(caps, xen)
 returns 0.

Yes, the autobuild failed on that too. I'll do a patch to disable those
two xen tests on platforms without support.

Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Question about more finer access control permission on libvirt

2008-05-16 Thread Daniel P. Berrange
On Fri, May 16, 2008 at 10:36:09AM +0900, Atsushi SAKAI wrote:
 Hi, Dan
 
 Thank you for commenting this.
 I am eased to hear this.
 I also agrees this issue has many task.
 
 p.s.
 I want to know the possibility of fine grained access control in libvirt,
 since our young guy is investigating the access control in Dom0-Xen.

For the libvirt MAC / fine grained ACL stuff I'm talking about, I don't want
us to build something that is tied / specific to Xen. The goal in the work 
should be to build the support such that it can reasonably apply to all the 
drivers in libvirt, so we get coverage across Xen, KVM, LXC, etc. Perhaps it
might integrate with the Xen  XSM support, but its too early to say whether 
XSM will be useful or not.  KVM of course is just a user space process like 
any other, so it is trivially secured with existing SELinux support. 

Regards,
Daniel.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] how to pass qemu options?

2008-05-16 Thread Daniel P. Berrange
On Thu, May 15, 2008 at 08:55:25PM +0200, Gerd Hoffmann wrote:
 Daniel P. Berrange wrote:
  is required for any serious development work.  I *hate* to having to
  create a wrapper script each time I need to pass in additional
  parameters, and I'd *love* to see libvirt being a bit more developer
  friendly.
  
  Historically we've not had very complete coverage of QEMU args, but we've
  been adding alot of new functionality recently so the need for extra args
  is reducing all the time. We recently added serial, parallel, sound and
  drive support. USB is the next on the list at which point we basically 
  have coverage of all the important options we should reasonably expect.
 
 Maybe for normal users.  Certainly not for developers.  I need stuff
 like raising debug level for more verbose logs, enable experimental
 features while working on them and similar stuff.  Nothing which I'd
 expect libvirt to support directly.
 
  Creating wrapper scripts for experimentation is not that difficult.
 
 But it is quite inconvenient.
 
 And it also proves that you can't prevent users from passing additional
 args to qemu, thereby creating unsupported configurations.  So what was
 the point in refusing emulator args=-foo.../emulator support?

There is a vast difference between having to go *outside* the API to add
extra args, vs supporting it directly in the XML format. 

Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] PATCH: Use -help arg with QEMU probing

2008-05-16 Thread Daniel P. Berrange
This patch makes the code that probes QEMU args explicitly add the -help
argument. This works around a problem with certain KVM builds which refuse
to run if executed without -help, and no KVM modules are present.

It also removes the set of setenv() since that changes libvirtd's environ
and instead uses execle() to set the child's environment


Dan

Index: src/qemu_conf.c
===
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.64
diff -u -p -r1.64 qemu_conf.c
--- src/qemu_conf.c 15 May 2008 20:07:34 -  1.64
+++ src/qemu_conf.c 16 May 2008 14:34:42 -
@@ -447,6 +447,8 @@ static int qemudExtractVersionInfo(const
 }
 
 if (child == 0) { /* Kid */
+const char *const qemuenv[] = { LANG=C, NULL };
+
 if (close(STDIN_FILENO)  0)
 goto cleanup1;
 if (close(STDERR_FILENO)  0)
@@ -457,8 +459,7 @@ static int qemudExtractVersionInfo(const
 goto cleanup1;
 
 /* Just in case QEMU is translated someday.. */
-setenv(LANG, C, 1);
-execl(qemu, qemu, (char*)NULL);
+execle(qemu, qemu, -help, (char*)NULL, qemuenv);
 
 cleanup1:
 _exit(-1); /* Just in case */

-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] PATCH: Don't rely on host binaries in QEMU tests

2008-05-16 Thread Daniel P. Berrange
The QEMU test suites rely on the QEMU/KVM/Xenner binaries being present
in /usr/bin. This has obvious problems and is unneccessary. The solution
is to not use the qemudCapsInit() function which initializes capabilities
based on binaries present. Instead I add a custom impl just for the test
cases which adds a pre-defined stable set of capabilities. I also had to
move a stat() check out of qemudBuildCommandLine() and into its caller.
It probably should have been there in the first place anyway

 src/qemu_conf.c  |   13 
 src/qemu_driver.c|   14 +
 tests/Makefile.am|4 +-
 tests/qemuxml2argvtest.c |4 +-
 tests/qemuxml2xmltest.c  |3 +
 tests/testutilsqemu.c|   71 +++
 tests/testutilsqemu.h|5 +++
 7 files changed, 97 insertions(+), 17 deletions(-)

Dan.

Index: src/qemu_conf.c
===
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.66
diff -u -p -u -p -r1.66 qemu_conf.c
--- src/qemu_conf.c 16 May 2008 09:37:44 -  1.66
+++ src/qemu_conf.c 16 May 2008 15:15:05 -
@@ -2326,7 +2326,6 @@ int qemudBuildCommandLine(virConnectPtr 
 char memory[50];
 char vcpus[50];
 char boot[QEMUD_MAX_BOOT_DEVS+1];
-struct stat sb;
 struct qemud_vm_disk_def *disk = vm-def-disks;
 struct qemud_vm_net_def *net = vm-def-nets;
 struct qemud_vm_input_def *input = vm-def-inputs;
@@ -2336,18 +2335,6 @@ int qemudBuildCommandLine(virConnectPtr 
 struct utsname ut;
 int disableKQEMU = 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),
Index: src/qemu_driver.c
===
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.75
diff -u -p -u -p -r1.75 qemu_driver.c
--- src/qemu_driver.c   16 May 2008 09:37:44 -  1.75
+++ src/qemu_driver.c   16 May 2008 15:15:05 -
@@ -646,6 +646,7 @@ static int qemudStartVMDaemon(virConnect
 char **argv = NULL, **tmp;
 int i, ret;
 char logfile[PATH_MAX];
+struct stat sb;
 
 if (qemudIsActiveVM(vm)) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -702,6 +703,19 @@ static int qemudStartVMDaemon(virConnect
 return -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;
+}
+
+
 if (qemudBuildCommandLine(conn, driver, vm, argv)  0) {
 close(vm-logfile);
 vm-logfile = -1;
Index: tests/Makefile.am
===
RCS file: /data/cvs/libvirt/tests/Makefile.am,v
retrieving revision 1.42
diff -u -p -u -p -r1.42 Makefile.am
--- tests/Makefile.am   28 Apr 2008 13:36:48 -  1.42
+++ tests/Makefile.am   16 May 2008 15:15:05 -
@@ -100,12 +100,12 @@ xmconfigtest_SOURCES = \
 xmconfigtest_LDADD = $(LDADDS)
 
 qemuxml2argvtest_SOURCES = \
-   qemuxml2argvtest.c \
+   qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h
 qemuxml2argvtest_LDADD = $(LDADDS)
 
 qemuxml2xmltest_SOURCES = \
-   qemuxml2xmltest.c \
+   qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h
 qemuxml2xmltest_LDADD = $(LDADDS)
 
Index: tests/qemuxml2argvtest.c
===
RCS file: /data/cvs/libvirt/tests/qemuxml2argvtest.c,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 qemuxml2argvtest.c
--- tests/qemuxml2argvtest.c15 May 2008 16:21:11 -  1.22
+++ tests/qemuxml2argvtest.c16 May 2008 15:15:05 -
@@ -14,6 +14,8 @@
 #include testutils.h
 #include qemu_conf.h
 
+#include testutilsqemu.h
+
 static char *progname;
 static char *abs_srcdir;
 static struct qemud_driver driver;
@@ -130,7 +132,7 @@ main(int argc, char **argv)
 if (!abs_srcdir)
 abs_srcdir = getcwd(cwd, sizeof(cwd));
 
-driver.caps = qemudCapsInit();
+

[libvirt] PATCH: Write pidfile earlier in startup

2008-05-16 Thread Daniel P. Berrange
When used with the --daemon arg, libvirtd will write out a pid file to
/var/run/libvirtd.pid and exit if this file already exists. Unfortuantely
it does this quite late in its startup procedure - in particular *after*
the call to qemudInitialize(), which in turns initializes the drivers,
which in turn autostarts all the VMs and networks.

So if you start a 2nd libvirtd instance it would do autostart before it
got to checking for existing PID file. This is clearly very bad because
the same VM would be started twice resulting in data corruption. Fortunately
the Red Hat / Fedora initscript also checks the PID file before even starting
the daemon, but this can affect people starting the daemon directly.

So this patch makes the goDaemon() / pidfile creation the very first thing
the daemon does after parsing command line arguments. This also results in
greatly increased startup time for OS, since the initscript doesn't have
to wait for it to auto-start all the VMs  networks before it daemonizes.
It also initializes the signal handlers before calling qemudInitialize()
since these really need to be setup before we start any VMs / child procs.

Dan.

Index: qemud/qemud.c
===
RCS file: /data/cvs/libvirt/qemud/qemud.c,v
retrieving revision 1.100
diff -u -p -r1.100 qemud.c
--- qemud/qemud.c   15 May 2008 06:12:32 -  1.100
+++ qemud/qemud.c   16 May 2008 15:58:43 -
@@ -2143,6 +2143,24 @@ int main(int argc, char **argv) {
 }
 }
 
+if (godaemon) {
+openlog(libvirtd, 0, 0);
+if (qemudGoDaemon()  0) {
+qemudLog(QEMUD_ERR, _(Failed to fork as daemon: %s),
+ strerror(errno));
+goto error1;
+}
+
+/* Choose the name of the PID file. */
+if (!pid_file) {
+if (REMOTE_PID_FILE[0] != '\0')
+pid_file = REMOTE_PID_FILE;
+}
+
+if (pid_file  qemudWritePidFile (pid_file)  0)
+goto error1;
+}
+
 if (pipe(sigpipe)  0 ||
 qemudSetNonBlock(sigpipe[0])  0 ||
 qemudSetNonBlock(sigpipe[1])  0 ||
@@ -2154,6 +2172,19 @@ int main(int argc, char **argv) {
 }
 sigwrite = sigpipe[1];
 
+sig_action.sa_sigaction = sig_handler;
+sig_action.sa_flags = SA_SIGINFO;
+sigemptyset(sig_action.sa_mask);
+
+sigaction(SIGHUP, sig_action, NULL);
+sigaction(SIGINT, sig_action, NULL);
+sigaction(SIGQUIT, sig_action, NULL);
+sigaction(SIGTERM, sig_action, NULL);
+sigaction(SIGCHLD, sig_action, NULL);
+
+sig_action.sa_handler = SIG_IGN;
+sigaction(SIGPIPE, sig_action, NULL);
+
 if (!(server = qemudInitialize(sigpipe[0]))) {
 ret = 2;
 goto error1;
@@ -2175,37 +2206,6 @@ int main(int argc, char **argv) {
  sockdirname);
 }
 
-if (godaemon) {
-openlog(libvirtd, 0, 0);
-if (qemudGoDaemon()  0) {
-qemudLog(QEMUD_ERR, _(Failed to fork as daemon: %s),
- strerror(errno));
-goto error1;
-}
-
-/* Choose the name of the PID file. */
-if (!pid_file) {
-if (REMOTE_PID_FILE[0] != '\0')
-pid_file = REMOTE_PID_FILE;
-}
-
-if (pid_file  qemudWritePidFile (pid_file)  0)
-goto error1;
-}
-
-sig_action.sa_sigaction = sig_handler;
-sig_action.sa_flags = SA_SIGINFO;
-sigemptyset(sig_action.sa_mask);
-
-sigaction(SIGHUP, sig_action, NULL);
-sigaction(SIGINT, sig_action, NULL);
-sigaction(SIGQUIT, sig_action, NULL);
-sigaction(SIGTERM, sig_action, NULL);
-sigaction(SIGCHLD, sig_action, NULL);
-
-sig_action.sa_handler = SIG_IGN;
-sigaction(SIGPIPE, sig_action, NULL);
-
 if (virEventAddHandleImpl(sigpipe[0],
   POLLIN,
   qemudDispatchSignalEvent,


-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Write pidfile earlier in startup

2008-05-16 Thread Stefan de Konink
On Fri, 16 May 2008, Daniel P. Berrange wrote:

 When used with the --daemon arg, libvirtd will write out a pid file to
 /var/run/libvirtd.pid and exit if this file already exists. Unfortuantely
 it does this quite late in its startup procedure - in particular *after*
 the call to qemudInitialize(), which in turns initializes the drivers,
 which in turn autostarts all the VMs and networks.

Does this also solve it when started with only -l?

Stefan

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Use -help arg with QEMU probing

2008-05-16 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 This patch makes the code that probes QEMU args explicitly add the -help
 argument. This works around a problem with certain KVM builds which refuse
 to run if executed without -help, and no KVM modules are present.

Sounds obtuse enough to deserve a comment in the code.

 It also removes the set of setenv() since that changes libvirtd's environ
 and instead uses execle() to set the child's environment


 Dan

 Index: src/qemu_conf.c
 ===
 RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
 retrieving revision 1.64
 diff -u -p -r1.64 qemu_conf.c
 --- src/qemu_conf.c   15 May 2008 20:07:34 -  1.64
 +++ src/qemu_conf.c   16 May 2008 14:34:42 -
 @@ -447,6 +447,8 @@ static int qemudExtractVersionInfo(const
  }

  if (child == 0) { /* Kid */
 +const char *const qemuenv[] = { LANG=C, NULL };
 +
  if (close(STDIN_FILENO)  0)
  goto cleanup1;
  if (close(STDERR_FILENO)  0)
 @@ -457,8 +459,7 @@ static int qemudExtractVersionInfo(const
  goto cleanup1;

  /* Just in case QEMU is translated someday.. */
 -setenv(LANG, C, 1);
 -execl(qemu, qemu, (char*)NULL);
 +execle(qemu, qemu, -help, (char*)NULL, qemuenv);

  cleanup1:
  _exit(-1); /* Just in case */

ACK

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Don't rely on host binaries in QEMU tests

2008-05-16 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 The QEMU test suites rely on the QEMU/KVM/Xenner binaries being present
 in /usr/bin. This has obvious problems and is unneccessary. The solution
 is to not use the qemudCapsInit() function which initializes capabilities
 based on binaries present. Instead I add a custom impl just for the test
 cases which adds a pre-defined stable set of capabilities. I also had to
 move a stat() check out of qemudBuildCommandLine() and into its caller.
 It probably should have been there in the first place anyway

  src/qemu_conf.c  |   13 
  src/qemu_driver.c|   14 +
  tests/Makefile.am|4 +-
  tests/qemuxml2argvtest.c |4 +-
  tests/qemuxml2xmltest.c  |3 +
  tests/testutilsqemu.c|   71 
 +++
  tests/testutilsqemu.h|5 +++

Looks fine.  I confirmed it does fix the test failures.
ACK.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Write pidfile earlier in startup

2008-05-16 Thread Daniel P. Berrange
On Fri, May 16, 2008 at 05:05:08PM +0100, Daniel P. Berrange wrote:
 When used with the --daemon arg, libvirtd will write out a pid file to
 /var/run/libvirtd.pid and exit if this file already exists. Unfortuantely
 it does this quite late in its startup procedure - in particular *after*
 the call to qemudInitialize(), which in turns initializes the drivers,
 which in turn autostarts all the VMs and networks.
 
 So if you start a 2nd libvirtd instance it would do autostart before it
 got to checking for existing PID file. This is clearly very bad because
 the same VM would be started twice resulting in data corruption. Fortunately
 the Red Hat / Fedora initscript also checks the PID file before even starting
 the daemon, but this can affect people starting the daemon directly.
 
 So this patch makes the goDaemon() / pidfile creation the very first thing
 the daemon does after parsing command line arguments. This also results in
 greatly increased startup time for OS, since the initscript doesn't have
 to wait for it to auto-start all the VMs  networks before it daemonizes.
 It also initializes the signal handlers before calling qemudInitialize()
 since these really need to be setup before we start any VMs / child procs.

There were actually some more changes needed. It needs to write the PID file
even if not running with --daemon mode to be truely safe, because some
init software won't run it in daemon mode at all. So this updated patch will
always write a PID file if run as root.  It also fixes a few flaws in the
cleanup process to ensure it only unlinks the pidfile on failure if it owned
the pidfile, and removes a pointless warning when running non-root.

Dan.

Index: qemud/qemud.c
===
RCS file: /data/cvs/libvirt/qemud/qemud.c,v
retrieving revision 1.100
diff -u -p -r1.100 qemud.c
--- qemud/qemud.c   15 May 2008 06:12:32 -  1.100
+++ qemud/qemud.c   16 May 2008 16:36:42 -
@@ -2143,6 +2143,26 @@ int main(int argc, char **argv) {
 }
 }
 
+if (godaemon) {
+openlog(libvirtd, 0, 0);
+if (qemudGoDaemon()  0) {
+qemudLog(QEMUD_ERR, _(Failed to fork as daemon: %s),
+ strerror(errno));
+goto error1;
+}
+}
+
+/* If running as root and no PID file is set, use the default */
+if (pid_file == NULL 
+getuid() == 0 
+REMOTE_PID_FILE[0] != '\0')
+pid_file = REMOTE_PID_FILE;
+
+/* If we have a pidfile set, claim it now, exiting if already taken */
+if (pid_file != NULL 
+qemudWritePidFile (pid_file)  0)
+goto error1;
+
 if (pipe(sigpipe)  0 ||
 qemudSetNonBlock(sigpipe[0])  0 ||
 qemudSetNonBlock(sigpipe[1])  0 ||
@@ -2150,24 +2170,34 @@ int main(int argc, char **argv) {
 qemudSetCloseExec(sigpipe[1])  0) {
 qemudLog(QEMUD_ERR, _(Failed to create pipe: %s),
  strerror(errno));
-goto error1;
+goto error2;
 }
 sigwrite = sigpipe[1];
 
+sig_action.sa_sigaction = sig_handler;
+sig_action.sa_flags = SA_SIGINFO;
+sigemptyset(sig_action.sa_mask);
+
+sigaction(SIGHUP, sig_action, NULL);
+sigaction(SIGINT, sig_action, NULL);
+sigaction(SIGQUIT, sig_action, NULL);
+sigaction(SIGTERM, sig_action, NULL);
+sigaction(SIGCHLD, sig_action, NULL);
+
+sig_action.sa_handler = SIG_IGN;
+sigaction(SIGPIPE, sig_action, NULL);
+
 if (!(server = qemudInitialize(sigpipe[0]))) {
 ret = 2;
-goto error1;
+goto error2;
 }
 
 /* Read the config file (if it exists). */
 if (remoteReadConfigFile (server, remote_config_file)  0)
-goto error1;
+goto error2;
 
 /* Change the group ownership of /var/run/libvirt to unix_sock_gid */
-if (getuid() != 0) {
-qemudLog (QEMUD_WARN,
-  %s, _(Cannot set group ownership when not running as 
root));
-} else {
+if (getuid() == 0) {
 const char *sockdirname = LOCAL_STATE_DIR /run/libvirt;
 
 if (chown(sockdirname, -1, unix_sock_gid)  0)
@@ -2175,37 +2205,6 @@ int main(int argc, char **argv) {
  sockdirname);
 }
 
-if (godaemon) {
-openlog(libvirtd, 0, 0);
-if (qemudGoDaemon()  0) {
-qemudLog(QEMUD_ERR, _(Failed to fork as daemon: %s),
- strerror(errno));
-goto error1;
-}
-
-/* Choose the name of the PID file. */
-if (!pid_file) {
-if (REMOTE_PID_FILE[0] != '\0')
-pid_file = REMOTE_PID_FILE;
-}
-
-if (pid_file  qemudWritePidFile (pid_file)  0)
-goto error1;
-}
-
-sig_action.sa_sigaction = sig_handler;
-sig_action.sa_flags = SA_SIGINFO;
-sigemptyset(sig_action.sa_mask);
-
-sigaction(SIGHUP, sig_action, NULL);
-sigaction(SIGINT, sig_action, NULL);
-sigaction(SIGQUIT, sig_action, 

[libvirt] [PATCH] plug two leaks

2008-05-16 Thread Jim Meyering
Running make check under valgrind exposed at least
the first.  Then, I spotted the other.


* src/qemu_conf.c (qemudParseXML): Free obj unconditionally.
---
 src/qemu_conf.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index ff7c63e..47d49a2 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1716,11 +1716,11 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr 
conn,
 (obj-stringval != NULL)  (obj-stringval[0] != 0)) {
 strncpy(def-os.bootloader, (const char*)obj-stringval, 
sizeof(def-os.bootloader));
 NUL_TERMINATE(def-os.bootloader);
-xmlXPathFreeObject(obj);

 /* Set a default OS type, since type is optional with bootloader */
 strcpy(def-os.type, xen);
 }
+xmlXPathFreeObject(obj);

 /* Extract OS type info */
 obj = xmlXPathEval(BAD_CAST string(/domain/os/type[1]), ctxt);
@@ -1733,9 +1733,9 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr 
conn,
 }
 } else {
 strcpy(def-os.type, (const char *)obj-stringval);
-xmlXPathFreeObject(obj);
-obj = NULL;
 }
+xmlXPathFreeObject(obj);
+obj = NULL;

 if (!virCapabilitiesSupportsGuestOSType(driver-caps, def-os.type)) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_OS_TYPE,
--
1.5.5.1.249.g26848

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] plug two leaks

2008-05-16 Thread Daniel P. Berrange
On Fri, May 16, 2008 at 06:44:48PM +0200, Jim Meyering wrote:
 Running make check under valgrind exposed at least
 the first.  Then, I spotted the other.

ACK.

Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] plug two leaks

2008-05-16 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 On Fri, May 16, 2008 at 06:44:48PM +0200, Jim Meyering wrote:
 Running make check under valgrind exposed at least
 the first.  Then, I spotted the other.

 ACK.

speedy ;-)
Committed.

Actually there's one more, but I don't have time for it right now:

 128 (96 direct, 32 indirect) bytes in 6 blocks are definitely lost in loss 
record 5 of 7
at 0x4A05174: calloc (vg_replace_malloc.c:397)
by 0x40A6EA: qemudParseVMDef (qemu_conf.c:2078)
by 0x402991: testCompareXMLToArgvHelper (qemuxml2argvtest.c:43)
by 0x4032C0: virtTestRun (testutils.c:79)
by 0x402861: main (qemuxml2argvtest.c:180)

It's allocated with this calloc call:

/* Parse sound driver xml */
obj = xmlXPathEval(BAD_CAST /domain/devices/sound, ctxt);
if ((obj != NULL)  (obj-type == XPATH_NODESET) 
(obj-nodesetval != NULL)  (obj-nodesetval-nodeNr = 0)) {
struct qemud_vm_sound_def *prev = NULL;
for (i = 0; i  obj-nodesetval-nodeNr; i++) {

struct qemud_vm_sound_def *sound = calloc(1, sizeof(*sound));
struct qemud_vm_sound_def *check = def-sounds;
int collision = 0;
if (!sound) {
qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
 %s, _(failed to allocate space for sound dev));
goto error;
}
if (qemudParseSoundXML(conn, sound,
   obj-nodesetval-nodeTab[i])  0) {
free(sound);
goto error;
}

// Check that model type isn't already present in sound dev list
while(check) {
if (check-model == sound-model) {
collision = 1;
break;
}
check = check-next;
}
if (collision)
continue;

def-nsounds++;
sound-next = NULL;
if (def-sounds == NULL) {
def-sounds = sound;
} else {
prev-next = sound;
}
prev = sound;
}
}

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Pass -name argument to QEMU

2008-05-16 Thread Soren Hansen
On Thu, May 15, 2008 at 10:24:35PM +0100, Daniel P. Berrange wrote:
 Xenner doesn't start its help output right at the start of line,

Ah. :/

 and there isn't any other output there that causes ambiguity in
 current qemu help output:
 
   # qemu-kvm | grep -- -drive
   -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][index=i]
 
 If one happens to be added in the future, that's fine because -drive
 will be enabled anyway for future releases.

I wasn't only worried about future releases, but also previous ones. I
didn't want to have to check back through qemu's vcs for instances of
-drive, so I coded defensively instead. :) What versions of QEmu does
libvirt actually claim to support? If we only support recent versions,
then we're probably fine.

-- 
Soren Hansen   | 
Virtualisation specialist  | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/


signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Free object after *Destroy in python bindings

2008-05-16 Thread Cole Robinson
The patch below fixes an issue in the python bindings with the
vir*Destroy methods. After the object is successfully destroyed,
the payload is cleared, using 'self._o = None'. This unfortunately
screws up virt object reference counts, as the payload should be 
free'd using the appropriate vir*Free function.

Thanks,
Cole
diff --git a/python/generator.py b/python/generator.py
index cb57bff..9421981 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -629,10 +629,10 @@ function_classes = {}
 function_classes[None] = []
 
 function_post = {
-'virDomainDestroy': self._o = None,
-'virNetworkDestroy': self._o = None,
-'virStoragePoolDestroy': self._o = None,
-'virStorageVolDestroy': self._o = None,
+'virDomainDestroy': del(self),
+'virNetworkDestroy': del(self),
+'virStoragePoolDestroy': del(self),
+'virStorageVolDestroy': del(self),
 }
 
 # Functions returning an integral type which need special rules to
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] PATCH: Support vCPU pinning in QEMU driver

2008-05-16 Thread Daniel P. Berrange
KVM added ability to get the thread ID for vCPUs via the monitor

(qemu) info cpus
* CPU #0: pc=0x0000 thread_id=11463
  CPU #1: pc=0xfff0 thread_id=11464
  CPU #2: pc=0xfff0 thread_id=11465

With this we have enough information to be able to support vCPU pinning in
the QEMU driver for KVM. For QEMU/KQEMU it is trivial, since they have a
single thread.

The following patch implements CPU pinning and fetching of CPU affinity
information.  In this example I pin one of the 2 cpus in a guest:

[EMAIL PROTECTED] libvirt-numa]$ ./src/virsh --connect qemu:///system start 
VirtTest
Domain VirtTest started

[EMAIL PROTECTED] libvirt-numa]$ ./src/virsh --connect qemu:///system vcpuinfo 
VirtTest
VCPU:   0
CPU:0
State:  running
CPU Affinity:   yy

VCPU:   1
CPU:0
State:  running
CPU Affinity:   yy

[EMAIL PROTECTED] libvirt-numa]$ ./src/virsh --connect qemu:///system vcpupin 
VirtTest 1 0

[EMAIL PROTECTED] libvirt-numa]$ ./src/virsh --connect qemu:///system vcpuinfo 
VirtTest
VCPU:   0
CPU:0
State:  running
CPU Affinity:   yy

VCPU:   1
CPU:0
State:  running
CPU Affinity:   y-



This is implemented using sched_setaffinity/sched_getaffinity which are 
Linux specific. There doesn't appear to be a portable process affinity
API in POSIX.

If the KVM instance does not support the 'thread_id' data in 'info cpus',
we simply print out a suitable error message. We detect the mapping at
startup and cache it thereafter.

Dan.

diff -r 0f537442ce97 src/qemu_conf.h
--- a/src/qemu_conf.h   Fri May 16 16:09:57 2008 -0400
+++ b/src/qemu_conf.h   Fri May 16 17:39:29 2008 -0400
@@ -328,6 +328,9 @@
 int *tapfds;
 int ntapfds;
 
+int nvcpupids;
+int *vcpupids;
+
 int qemuVersion;
 int qemuCmdFlags; /* values from enum qemud_cmd_flags */
 
diff -r 0f537442ce97 src/qemu_driver.c
--- a/src/qemu_driver.c Fri May 16 16:09:57 2008 -0400
+++ b/src/qemu_driver.c Fri May 16 17:39:29 2008 -0400
@@ -61,6 +61,7 @@
 #include nodeinfo.h
 #include stats_linux.h
 #include capabilities.h
+#include memory.h
 
 static int qemudShutdown(void);
 
@@ -118,6 +119,10 @@
   struct qemud_network *network);
 
 static int qemudDomainGetMaxVcpus(virDomainPtr dom);
+static int qemudMonitorCommand (const struct qemud_driver *driver,
+const struct qemud_vm *vm,
+const char *cmd,
+char **reply);
 
 static struct qemud_driver *qemu_driver = NULL;
 
@@ -608,6 +613,106 @@
 return ret;
 }
 
+static int
+qemudDetectVcpuPIDs(virConnectPtr conn,
+struct qemud_driver *driver,
+struct qemud_vm *vm) {
+char *qemucpus = NULL;
+char *line;
+int lastVcpu = -1;
+
+/* Only KVM has seperate threads for CPUs,
+   others just use main QEMU process for CPU */
+if (vm-def-virtType != QEMUD_VIRT_KVM)
+vm-nvcpupids = 1;
+else
+vm-nvcpupids = vm-def-vcpus;
+
+if (VIR_ALLOC_N(vm-vcpupids, vm-nvcpupids)  0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
+ %s, _(allocate cpumap));
+return -1;
+}
+
+if (vm-def-virtType != QEMUD_VIRT_KVM) {
+vm-vcpupids[0] = vm-pid;
+return 0;
+}
+
+if (qemudMonitorCommand(driver, vm, info cpus, qemucpus)  0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(cannot run monitor command to fetch CPU 
thread info));
+VIR_FREE(vm-vcpupids);
+vm-nvcpupids = 0;
+return -1;
+}
+
+/*
+ * This is the gross format we're about to parse :-{
+ *
+ * (qemu) info cpus
+ * * CPU #0: pc=0x000f0c4a thread_id=30019
+ *   CPU #1: pc=0xfff0 thread_id=30020
+ *   CPU #2: pc=0xfff0 thread_id=30021
+ *
+ */
+line = qemucpus;
+do {
+char *offset = strchr(line, '#');
+char *end = NULL;
+int vcpu = 0, tid = 0;
+
+/* See if we're all done */
+if (offset == NULL)
+break;
+
+/* Extract VCPU number */
+if (virStrToLong_i(offset + 1, end, 10, vcpu)  0)
+goto error;
+if (end == NULL || *end != ':')
+goto error;
+
+/* Extract host Thread ID */
+if ((offset = strstr(line, thread_id=)) == NULL)
+goto error;
+if (virStrToLong_i(offset + strlen(thread_id=), end, 10, tid)  0)
+goto error;
+if (end == NULL || !c_isspace(*end))
+goto error;
+
+/* Validate the VCPU is in expected range  order */
+if (vcpu  vm-nvcpupids ||
+vcpu != (lastVcpu + 1))
+goto error;
+
+lastVcpu = vcpu;
+vm-vcpupids[vcpu] = tid;
+
+/* Skip to next data line */
+line = 

[libvirt] PATCH: Support initial boot time CPU affinity mask

2008-05-16 Thread Daniel P. Berrange
The XML format allows for an initial CPU mask to be specified for a guests
vCPUs. eg with this XML:

  vcpu cpuset='1-4,8-20,525'1/vcpu

Since we have CPU pinning support from my previous patch, adding in the
initial pinning is fairly easy. We first pass the '-S' arg to QEMU when
forking it. This causes it to initialize, but not start the CPUs in the
guest. We then set the affinity mask for all its CPUs, and then send 
the 'cont' command to the monitor to start execution.


 src/qemu_conf.c|   44 +++
 src/qemu_conf.h|3 
 src/qemu_driver.c  |   77 +
 src/xml.c  |5 
 src/xml.h  |4 
 tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-boot-network.args  |2 
 tests/qemuxml2argvdata/qemuxml2argv-bootloader.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args |2 
 tests/qemuxml2argvdata/qemuxml2argv-console-compat.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-disk-many.args |2 
 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args  |2 
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.args  |2 
 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-input-xen.args |2 
 tests/qemuxml2argvdata/qemuxml2argv-minimal.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-minimal.xml|2 
 tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.args |2 
 tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-net-user.args  |2 
 tests/qemuxml2argvdata/qemuxml2argv-net-virtio.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.args  |2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-dev.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-file.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-many.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-pty.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.args |2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-udp.args|2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-unix.args   |2 
 tests/qemuxml2argvdata/qemuxml2argv-serial-vc.args |2 
 tests/qemuxml2argvdata/qemuxml2argv-sound.args |2 
 39 files changed, 140 insertions(+), 61 deletions(-)

Dan.

diff -r 3bbea433803f src/qemu_conf.c
--- a/src/qemu_conf.c   Fri May 16 17:39:29 2008 -0400
+++ b/src/qemu_conf.c   Fri May 16 17:40:39 2008 -0400
@@ -56,6 +56,7 @@
 #include memory.h
 #include verify.h
 #include c-ctype.h
+#include xml.h
 
 #define qemudLog(level, msg...) fprintf(stderr, msg)
 
@@ -1743,6 +1744,25 @@
 }
 xmlXPathFreeObject(obj);
 
+/* Extract domain vcpu info */
+obj = xmlXPathEval(BAD_CAST string(/domain/vcpu[1]/@cpuset), ctxt);
+if ((obj == NULL) || (obj-type != XPATH_STRING) ||
+(obj-stringval == NULL) || (obj-stringval[0] == 0)) {
+/* Allow use on all CPUS */
+memset(def-cpumask, 1, QEMUD_CPUMASK_LEN);
+} else {
+char *set = (char *)obj-stringval;
+memset(def-cpumask, 0, QEMUD_CPUMASK_LEN);
+if (virParseCpuSet(conn, (const char **)set,
+   0, def-cpumask,
+   QEMUD_CPUMASK_LEN)  0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(malformed vcpu mask information));
+goto error;
+}
+}
+xmlXPathFreeObject(obj);
+
 /* See if ACPI feature is requested */
 obj = xmlXPathEval(BAD_CAST /domain/features/acpi, ctxt);
 if ((obj != NULL)  (obj-type == XPATH_NODESET) 
@@ -2431,6 +2451,7 @@
 disableKQEMU = 1;
 
 len = 1 + /* qemu */
+1 + /* Stopped */
 2 + /* machine type */
 disableKQEMU + /* Disable kqemu */
 (vm-qemuCmdFlags  QEMUD_CMD_FLAG_NAME ? 2 : 0) + /* -name XXX */
@@ -2464,6 +2485,8 @@
 goto no_memory;
 if (!((*argv)[++n] = strdup(vm-def-os.binary)))
 goto no_memory;
+if (!((*argv)[++n] = strdup(-S)))
+goto no_memory;
 if (!((*argv)[++n] = 

Re: [libvirt] PATCH: Support initial boot time CPU affinity mask

2008-05-16 Thread Stefan de Konink
On Fri, 16 May 2008, Daniel P. Berrange wrote:

 The XML format allows for an initial CPU mask to be specified for a guests
 vCPUs. eg with this XML:

   vcpu cpuset='1-4,8-20,525'1/vcpu

 Since we have CPU pinning support from my previous patch, adding in the
 initial pinning is fairly easy. We first pass the '-S' arg to QEMU when
 forking it. This causes it to initialize, but not start the CPUs in the
 guest. We then set the affinity mask for all its CPUs, and then send
 the 'cont' command to the monitor to start execution.

Does this include Xen support?


Stefan

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Support initial boot time CPU affinity mask

2008-05-16 Thread Cole Robinson
Stefan de Konink wrote:
 On Fri, 16 May 2008, Daniel P. Berrange wrote:
 
 The XML format allows for an initial CPU mask to be specified for a guests
 vCPUs. eg with this XML:

   vcpu cpuset='1-4,8-20,525'1/vcpu

 Since we have CPU pinning support from my previous patch, adding in the
 initial pinning is fairly easy. We first pass the '-S' arg to QEMU when
 forking it. This causes it to initialize, but not start the CPUs in the
 guest. We then set the affinity mask for all its CPUs, and then send
 the 'cont' command to the monitor to start execution.
 
 Does this include Xen support?
 
 
 Stefan
 

cpu pinning is already support for xen.

- Cole

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Use -help arg with QEMU probing

2008-05-16 Thread Anthony Liguori

Jim Meyering wrote:

Daniel P. Berrange [EMAIL PROTECTED] wrote:

This patch makes the code that probes QEMU args explicitly add the -help
argument. This works around a problem with certain KVM builds which refuse
to run if executed without -help, and no KVM modules are present.


Sounds obtuse enough to deserve a comment in the code.


In all fairness, it's never been well defined that the help script is 
outputted when no options are passed.  It actually is machine dependent.


When the config file support is merged, and we introduce a default 
config, it will be possible to configure QEMU to default to -net tap and 
-boot cdn.  In this case, a VM will be launched if no arguments are passed.


Regards,

Anthony Liguori


It also removes the set of setenv() since that changes libvirtd's environ
and instead uses execle() to set the child's environment


Dan

Index: src/qemu_conf.c
===
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.64
diff -u -p -r1.64 qemu_conf.c
--- src/qemu_conf.c 15 May 2008 20:07:34 -  1.64
+++ src/qemu_conf.c 16 May 2008 14:34:42 -
@@ -447,6 +447,8 @@ static int qemudExtractVersionInfo(const
 }

 if (child == 0) { /* Kid */
+const char *const qemuenv[] = { LANG=C, NULL };
+
 if (close(STDIN_FILENO)  0)
 goto cleanup1;
 if (close(STDERR_FILENO)  0)
@@ -457,8 +459,7 @@ static int qemudExtractVersionInfo(const
 goto cleanup1;

 /* Just in case QEMU is translated someday.. */
-setenv(LANG, C, 1);
-execl(qemu, qemu, (char*)NULL);
+execle(qemu, qemu, -help, (char*)NULL, qemuenv);

 cleanup1:
 _exit(-1); /* Just in case */


ACK



--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Support initial boot time CPU affinity mask

2008-05-16 Thread Jim Paris
Daniel P. Berrange wrote:
 Since we have CPU pinning support from my previous patch, adding in the
 initial pinning is fairly easy. We first pass the '-S' arg to QEMU when
 forking it.

If -S is always added, this becomes unnecessary (qemu_conf.c:2823):

if (vm-migrateFrom[0]) {
if (!((*argv)[++n] = strdup(-S)))
goto no_memory;

-jim

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list