[libvirt] VolumeCreateXML XML description for ESX
Hello, I am using libvirt 0.8.6 python bindings can someone point me to where i find the XML description of creating volumes using storagecolumecreateXML function ? i am getting libvir: ESX error : internal error Volume name 'name.vmdk' doesn't have expected format 'directory/file' thank you regards, Sherif -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] make syntax-check - make: *** [sc_check_author_list] Error 1 on libvirt-0.8.5
Best Regards, Kenneth Nagin Ph: +972-4-8296227 Cell: 054-6976227 Fx: +972-4- 8296113 https://researcher.ibm.com/researcher/view.php?person=il-NAGIN http://www.reservoir-fp7.eu/ A lie can travel halfway round the world while the truth is putting on its shoes. -- Mark Twain Eric Blake ebl...@redhat.com wrote on 01/12/2010 18:05:46: From: Eric Blake ebl...@redhat.com To: Justin Clift jcl...@redhat.com Cc: Kenneth Nagin/Haifa/i...@ibmil, list libvirt libvir-list@redhat.com Date: 01/12/2010 18:05 Subject: Re: [libvirt] make syntax-check - make: *** [sc_check_author_list] Error 1 on libvirt-0.8.5 On 12/01/2010 04:14 AM, Justin Clift wrote: On 01/12/2010, at 9:03 PM, Kenneth Nagin wrote: I am receiving syntax error when compiling libvirt-0.8.5. However, make without syntax-check completes successfully. Interesting. There was a problem with make syntax-check that was patched last night, after the 0.8.6 release. Yes, but it was a different check that was failing, and the patch for that fix won't solve the too-old git failing the authors check. If you can, and the error still crops up, then it's a new one we need to look at. Otherwise it's already been fixed in the source, your choice of which source tarball version you want to then use. :) And ultimately, failure of 'make syntax-check' is non-fatal; it is not a prerequisite for building working binaries, so much as a way to try and enforce consistent style within the code base. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org [attachment signature.asc deleted by Kenneth Nagin/Haifa/IBM] I decided that it made more sense to simply work with 0.8.6 (rather than 0.8.5). But now I am getting another error, i.e Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6. It prints it a lot of times and then hangs. make without syntax-check works fine. Here are the error messages: na...@croton:~/LIBVIRT/libvirt-0.8.6$ make syntax-check GFDL_version ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 GFDL_version GPL_version ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 GPL_version Wundef_boolean 0.01 Wundef_boolean avoid_if_before_free ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 useless-if-before-free: missing FILE argument Try `useless-if-before-free --help' for more information. 0.03 avoid_if_before_free bindtextdomain ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 bindtextdomain cast_of_alloca_return_value ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 cast_of_alloca_return_value cast_of_argument_to_free ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 cast_of_argument_to_free cast_of_x_alloc_return_value ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 cast_of_x_alloc_return_value changelog ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 0.01 changelog const_long_option ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] schemas: Fix cpu element schema
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index fb44335..08ebefb 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1745,6 +1745,8 @@ ref name=cpuModel/ optional ref name=cpuVendor/ +/optional +optional ref name=cpuTopology/ ACK; makes it so that either one can be used in isolation, instead of a both-or-none approach. Thanks, I pushed the patch. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Add tests for CPU selection in qemu driver
Jiri Denemark (2): tests: Support for faking emulator in qemuxml2argv tests: Add tests for CPU selection in qemu driver I made the change requested by Eric and pushed both patches. Thanks for the review. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VolumeCreateXML XML description for ESX
On 02/12/2010, at 8:18 PM, Sherif Nagy wrote: Hello, I am using libvirt 0.8.6 python bindings can someone point me to where i find the XML description of creating volumes using storagecolumecreateXML function ? i am getting libvir: ESX error : internal error Volume name 'name.vmdk' doesn't have expected format 'directory/file' Hi Sherif, In theory, it's probably supposed to work with the standard storage and pool XML format documented here: http://libvirt.org/formatstorage.html But, it sounds like in practise that's not working for you. Is that the case? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/5] Add a watchdog action `dump'
On Wed, Dec 01, 2010 at 06:39:14PM -0700, Eric Blake wrote: On 12/01/2010 05:50 PM, Hu Tao wrote: On Tue, Nov 30, 2010 at 03:21:36PM -0700, Eric Blake wrote: ...snip... -libvirt_qemu_la_SOURCES = libvirt-qemu.c +libvirt_qemu_la_SOURCES = libvirt-qemu.c util/threadpool.c Why is this change necessary? Shouldn't libvirt_util.la already include threadpool.c, and the qemu driver already be linking with libvirt_util.la? Is this ok? -libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) +libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) ../src/libvirt_util.la Nope; rather... Or link will fail: CCLD libvirtd libvirtd-libvirtd.o: In function `qemudRunLoop': /mnt/data/kernel/libvirt/daemon/libvirtd.c:2229: undefined reference to `virWorkerPoolNew' /mnt/data/kernel/libvirt/daemon/libvirtd.c:2287: undefined reference to `virWorkerPoolFree' That means you need to modify src/libvirt_private.syms to export the new public interfaces from threadpool.h (it should be pretty easy to figure out what edits to make, the tricky part is realizing you need to touch that file in the first place). +if (virAsprintf(qemu_driver-autoDumpPath, %s/qemu/dump, base) == -1) +goto out_of_memory; However, it does raise an issue. Is qemu.conf only for privileged users, or do we have to worry about allowing non-privileged users also be able to pick up an alternate directory (especially since they can't dump to /var/log/...)? qemu.conf is only for privileged users, but non-privileged users who need to analyze dump files should ask admin to specify an auto-dump directory they have right to read. Or do you have a better idea? Is it better to dump a non-privileged log into ~/.libvirt/qemu/dump, so that it's automatically user-accessible? We already use ~/.libvirt for other non-privileged files. I think you're mixing up unprivileged users using the privileged qemu:///system, with unprivileged users using the unprivileged driver qemu://session. Only the latter uses ~/.libvirt and this patch should already be using ~/.libvirt/qemu/dump in that scenario. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
On Thu, Dec 02, 2010 at 02:47:22AM +1100, Justin Clift wrote: On 02/12/2010, at 2:26 AM, Diego Elio Pettenò wrote: Il giorno mar, 30/11/2010 alle 21.09 +0100, Daniel Veillard ha scritto: As indicated 10 days ago, today was time for a release, I didn't had much time so I simply generated a release from libvirt git without much testing. Hopefully this will be okay ! Doesn't seem to be the case :/ Ouch, that's two releases in a row with problems showing up straight away after release. Looks like it might be time to put some kind of regression testing in place, as a go/no-go release criteria. Our adhoc approach isn't working well enough any more. If we had followed our existing rules more strictly it would be have been fine. The stuff that caused the breakage was a new feature, so it should not have been merged at that late stage. All new features should have been merged a week before, with only bug fixes following. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
On Thu, Dec 02, 2010 at 05:47:01AM +1100, Justin Clift wrote: On 02/12/2010, at 5:26 AM, Diego Elio Pettenò wrote: Il giorno gio, 02/12/2010 alle 02.47 +1100, Justin Clift ha scritto: Looks like it might be time to put some kind of regression testing in place, as a go/no-go release criteria. May I suggest a 1-week (or less) window without merge of new features/improvements, announced on a separate (low-traffic) mailinglist for packagers to test the release? We'd then have time to test whether the code is fine for all of us or not. Concept wise, do you reckon something like this would work: + a new libvirt-announce mailing list, low trafic, purely for release announcements and similar Along with us announcing a 'release candidate build through it (instead of the present approach). If it looks good after a period of time (a week or something as you mentioned), then it gets re-released as the actual release. If something turns up significantly broken, then we respin as a release candidate 2 and repeat the process. I think this is really viable, because it implies we need another week prior to creating the pre-release where we do what we currently do with pre-release stabalization. With a monthly release cycle, taking 2 weeks todo a release is too much of an time sink. IMHO, we need to have - A official list of supported platforms / OS combinations - Run a test build on each combination before release - Actually follow the 'bug fixes' only rule leading upto release no matter how simple the new feature might appear. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
On 02/12/2010, at 9:58 PM, Daniel P. Berrange wrote: snip I think this is really viable, because it implies we need another Err... not really viable yeah? week prior to creating the pre-release where we do what we currently do with pre-release stabalization. With a monthly release cycle, taking 2 weeks todo a release is too much of an time sink. IMHO, we need to have - A official list of supported platforms / OS combinations Yep, good idea. We should definitely have a list of core things we support, plus some way of communicating things also being worked up. - Run a test build on each combination before release Yep. - Actually follow the 'bug fixes' only rule leading upto release no matter how simple the new feature might appear. Would it make sense to branch git at the point we enter feature freeze, having the new branch be just for the release in question, and allow people to continue committing to master? (I'd also think this is the point we could generate a release candidate tarball for people to test if they want) When bugs turn up in the freeze period, the fixes can be applied to the release branch. It sounds like it _might_ also mean a bit more work, as the same fixes will need to be applied to the master branch too. Good/bad approach? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VolumeCreateXML XML description for ESX
yes i am using same template like KVM but it is not working, seems ESX driver has a different XML structure since it is using datatstore /dir/filename.vdk structure. so any idea what is the XML structure for creating the XML for volumes using the ESX driver? On Thu, Dec 2, 2010 at 12:48 PM, Justin Clift jcl...@redhat.com wrote: On 02/12/2010, at 8:18 PM, Sherif Nagy wrote: Hello, I am using libvirt 0.8.6 python bindings can someone point me to where i find the XML description of creating volumes using storagecolumecreateXML function ? i am getting libvir: ESX error : internal error Volume name 'name.vmdk' doesn't have expected format 'directory/file' Hi Sherif, In theory, it's probably supposed to work with the standard storage and pool XML format documented here: http://libvirt.org/formatstorage.html But, it sounds like in practise that's not working for you. Is that the case? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VolumeCreateXML XML description for ESX
Matthias, sounds like there's a bug or we need to update the docs? On 02/12/2010, at 11:02 PM, Sherif Nagy wrote: yes i am using same template like KVM but it is not working, seems ESX driver has a different XML structure since it is using datatstore /dir/filename.vdk structure. so any idea what is the XML structure for creating the XML for volumes using the ESX driver? On Thu, Dec 2, 2010 at 12:48 PM, Justin Clift jcl...@redhat.com wrote: On 02/12/2010, at 8:18 PM, Sherif Nagy wrote: Hello, I am using libvirt 0.8.6 python bindings can someone point me to where i find the XML description of creating volumes using storagecolumecreateXML function ? i am getting libvir: ESX error : internal error Volume name 'name.vmdk' doesn't have expected format 'directory/file' Hi Sherif, In theory, it's probably supposed to work with the standard storage and pool XML format documented here: http://libvirt.org/formatstorage.html But, it sounds like in practise that's not working for you. Is that the case? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VolumeCreateXML XML description for ESX
The XML i am trying to use is volumenametest_vm.vmdk/namekey/keyallocation0/allocationcapacity unit='G'2/capacity/volume or volumenametest_vm.vmdk/namedirectorytest_vmfiletest_vm.vmdk/file/directorykey/keyallocation0/allocationcapacity unit='G'2/capacity/volume and if i added directory and file directive i am still getting the same error, i am not sure if i am doing something wrong or it is a bug , can someone please advice me what is the correct XML structure for creating volume using the ESX driver ? Thank You Regards, Sherif On Thu, Dec 2, 2010 at 2:04 PM, Justin Clift jcl...@redhat.com wrote: Matthias, sounds like there's a bug or we need to update the docs? On 02/12/2010, at 11:02 PM, Sherif Nagy wrote: yes i am using same template like KVM but it is not working, seems ESX driver has a different XML structure since it is using datatstore /dir/filename.vdk structure. so any idea what is the XML structure for creating the XML for volumes using the ESX driver? On Thu, Dec 2, 2010 at 12:48 PM, Justin Clift jcl...@redhat.com wrote: On 02/12/2010, at 8:18 PM, Sherif Nagy wrote: Hello, I am using libvirt 0.8.6 python bindings can someone point me to where i find the XML description of creating volumes using storagecolumecreateXML function ? i am getting libvir: ESX error : internal error Volume name 'name.vmdk' doesn't have expected format 'directory/file' Hi Sherif, In theory, it's probably supposed to work with the standard storage and pool XML format documented here: http://libvirt.org/formatstorage.html But, it sounds like in practise that's not working for you. Is that the case? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Thread pool impl
On Thu, Dec 02, 2010 at 09:43:12AM +0800, Hu Tao wrote:
On Wed, Dec 01, 2010 at 05:26:27PM +, Daniel P. Berrange wrote:
From: Hu Tao hu...@cn.fujitsu.com
* src/util/threadpool.c, src/util/threadpool.h: Thread pool
implementation
* src/Makefile.am: Build thread pool
---
src/Makefile.am |1 +
src/util/threadpool.c | 178
+
src/util/threadpool.h | 23 ++
3 files changed, 202 insertions(+), 0 deletions(-)
create mode 100644 src/util/threadpool.c
create mode 100644 src/util/threadpool.h
+static void virThreadPoolWorker(void *opaque)
+{
+virThreadPoolPtr pool = opaque;
+
+virMutexLock(pool-mutex);
+
+while (1) {
+while (!pool-quit
+ !pool-jobList) {
+pool-freeWorkers++;
+if (virCondWait(pool-cond, pool-mutex) 0) {
+pool-freeWorkers--;
+break;
+}
+pool-freeWorkers--;
+}
+
+if (pool-quit)
+break;
+
+virThreadPoolJobPtr job = pool-jobList;
+pool-jobList = pool-jobList-next;
+job-next = NULL;
+
+virMutexUnlock(pool-mutex);
+(pool-jobFunc)(job-data, pool-jobOpaque);
This could race if jobFunc does something with jobOpaque unless jobFunc
is aware of this and provides a lock to protect jobOpaque.
Yes, it is up to jobFunc to provide locking on jobOpaque if
it needs to do so for thread safety.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 1/4] threadpool impl
On Thu, Dec 02, 2010 at 03:26:57PM +0800, Hu Tao wrote:
+virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
+ size_t maxWorkers,
+ virThreadPoolJobFunc func,
+ void *opaque)
+{
+virThreadPoolPtr pool;
+size_t i;
+
+if (minWorkers maxWorkers)
+minWorkers = maxWorkers;
+
+if (VIR_ALLOC(pool) 0) {
+virReportOOMError();
+return NULL;
+}
+
+pool-jobList.head = NULL;
+pool-jobList.tail = pool-jobList.head;
+
+pool-jobFunc = func;
+pool-jobOpaque = opaque;
+
+if (virMutexInit(pool-mutex) 0)
+goto error;
+if (virCondInit(pool-cond) 0)
+goto error;
+if (virCondInit(pool-quit_cond) 0)
+goto error;
+
+if (VIR_ALLOC_N(pool-workers, minWorkers) 0)
+goto error;
+
+pool-maxWorkers = maxWorkers;
+for (i = 0; i minWorkers; i++) {
+if (virThreadCreate(pool-workers[i],
+true,
+virThreadPoolWorker,
+pool) 0) {
+virThreadPoolFree(pool);
+return NULL;
+}
+pool-nWorkers++;
+}
+
+return pool;
+
+error:
+VIR_FREE(pool-workers);
+ignore_value(virCondDestroy(pool-quit_cond));
+ignore_value(virCondDestroy(pool-cond));
+virMutexDestroy(pool-mutex);
+return NULL;
+}
This is leaking 'pool' on error. IMHO it is preferrable to
just call virThreadPoolDestroy, otherwise anytime we add
another field to virThreadPoolPtr struct, we have to consider
updating 2 cleanup paths.
+
+void virThreadPoolFree(virThreadPoolPtr pool)
+{
+virThreadPoolJobPtr job;
+
+if (!pool)
+return;
+
+virMutexLock(pool-mutex);
+pool-quit = true;
+if (pool-nWorkers 0) {
+virCondBroadcast(pool-cond);
+ignore_value(virCondWait(pool-quit_cond, pool-mutex));
+}
+
+while ((job = pool-jobList.head)) {
+pool-jobList.head = pool-jobList.head-next;
+VIR_FREE(job);
+}
+
+VIR_FREE(pool-workers);
+virMutexUnlock(pool-mutex);
+virMutexDestroy(pool-mutex);
+ignore_value(virCondDestroy(pool-quit_cond));
+ignore_value(virCondDestroy(pool-cond));
+VIR_FREE(pool);
+}
+
+int virThreadPoolSendJob(virThreadPoolPtr pool,
+ void *jobData)
+{
+virThreadPoolJobPtr job;
+
+virMutexLock(pool-mutex);
+if (pool-quit)
+goto error;
+
+if (pool-freeWorkers == 0
+pool-nWorkers pool-maxWorkers) {
+if (VIR_EXPAND_N(pool-workers, pool-nWorkers, 1) 0) {
+virReportOOMError();
+goto error;
+}
+
+if (virThreadCreate(pool-workers[pool-nWorkers - 1],
+true,
+virThreadPoolWorker,
+pool) 0) {
+pool-nWorkers--;
+goto error;
+}
Small typo, that check should != NULL, rather than 0.
+}
+
+if (VIR_ALLOC(job) 0) {
+virReportOOMError();
+goto error;
+}
+
+job-data = jobData;
+job-next = NULL;
+*pool-jobList.tail = job;
+pool-jobList.tail = (*pool-jobList.tail)-next;
+
+virCondSignal(pool-cond);
+virMutexUnlock(pool-mutex);
+
+return 0;
+
+error:
+virMutexUnlock(pool-mutex);
+return -1;
+}
diff --git a/src/util/threadpool.h b/src/util/threadpool.h
new file mode 100644
index 000..9ff27ec
--- /dev/null
+++ b/src/util/threadpool.h
@@ -0,0 +1,49 @@
+#ifndef __VIR_THREADPOOL_H__
+#define __VIR_THREADPOOL_H__
+
+#include threads.h
There's no need to include threads.h here since no virThread
stuff is exposed in the API. Just use internal.h instead.
+
+typedef struct _virThreadPool virThreadPool;
+typedef virThreadPool *virThreadPoolPtr;
+
+typedef void (*virThreadPoolJobFunc)(void *jobdata, void *opaque);
+
+virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
+ size_t maxWorkers,
+ virThreadPoolJobFunc func,
+ void *opaque) ATTRIBUTE_NONNULL(3)
+ATTRIBUTE_RETURN_CHECK;
ATTRIBUTE_RETURN_CHECK doesn't serve any useful purpose
when placed on constructors, since the caller will always
use the return value by assigning the pointer to some
variable. The compiler can thus never detect whether you
check for null or not, even with this annotation.
+void virThreadPoolFree(virThreadPoolPtr pool);
+
+int virThreadPoolSendJob(virThreadPoolPtr pool,
+ void *jobdata) ATTRIBUTE_NONNULL(1)
+ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_RETURN_CHECK;
Regards,
Daniel
--
libvir-list mailing
Re: [libvirt] VolumeCreateXML XML description for ESX
This XML snippet should work volume nametest_vm/test_vm.vmdk/name allocation0/allocation capacity unit='G'2/capacity /volume The error message says that the volume name doesn't have the expected format directory/file. The might be misleading here, they don't refer to XML elements. I can probably relax this and allow files in the datastore root. The problem with a .vmdk file in the datastore root is that ESX doesn't allow a virtual machine to be registered (or defined in libvirt terms) in the datastore root. The typical layout is to have a subdirectory per virtual machine. Matthias 2010/12/2 Sherif Nagy sherif.n...@gmail.com: The XML i am trying to use is volumenametest_vm.vmdk/namekey/keyallocation0/allocationcapacity unit='G'2/capacity/volume or volumenametest_vm.vmdk/namedirectorytest_vmfiletest_vm.vmdk/file/directorykey/keyallocation0/allocationcapacity unit='G'2/capacity/volume and if i added directory and file directive i am still getting the same error, i am not sure if i am doing something wrong or it is a bug , can someone please advice me what is the correct XML structure for creating volume using the ESX driver ? Thank You Regards, Sherif On Thu, Dec 2, 2010 at 2:04 PM, Justin Clift jcl...@redhat.com wrote: Matthias, sounds like there's a bug or we need to update the docs? On 02/12/2010, at 11:02 PM, Sherif Nagy wrote: yes i am using same template like KVM but it is not working, seems ESX driver has a different XML structure since it is using datatstore /dir/filename.vdk structure. so any idea what is the XML structure for creating the XML for volumes using the ESX driver? On Thu, Dec 2, 2010 at 12:48 PM, Justin Clift jcl...@redhat.com wrote: On 02/12/2010, at 8:18 PM, Sherif Nagy wrote: Hello, I am using libvirt 0.8.6 python bindings can someone point me to where i find the XML description of creating volumes using storagecolumecreateXML function ? i am getting libvir: ESX error : internal error Volume name 'name.vmdk' doesn't have expected format 'directory/file' Hi Sherif, In theory, it's probably supposed to work with the standard storage and pool XML format documented here: http://libvirt.org/formatstorage.html But, it sounds like in practise that's not working for you. Is that the case? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 3/4] Add a watchdog action `dump'
On Thu, Dec 02, 2010 at 03:30:10PM +0800, Hu Tao wrote:
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f4f965e..ba41f80 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -191,6 +191,11 @@
# save_image_format = raw
# dump_image_format = raw
+# When a domain is configured to be auto-dumped when libvirtd receives a
+# watchdog event from qemu guest, libvirtd will save dump files in directory
+# specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump
+#
+# auto_dump_path = /var/lib/libvirt/qemu/dump
# If provided by the host and a hugetlbfs mount point is configured,
# a guest may request huge page backing. When this mount point is
Also need to list this new setting in qemu.aug and test_qemu.aug
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e534195..bd25d90 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6263,6 +6300,64 @@ cleanup:
return ret;
}
+static void processWatchdogEvent(void *data, void *opaque ATTRIBUTE_UNUSED)
+{
+int ret;
+struct watchdogEvent *wdEvent = data;
+
+switch (wdEvent-action) {
+case VIR_DOMAIN_WATCHDOG_ACTION_DUMP:
+{
+char *dumpfile;
+int i;
+
+qemuDomainObjPrivatePtr priv = wdEvent-vm-privateData;
+
+i = virAsprintf(dumpfile, %s/%s-%u,
+qemu_driver-autoDumpPath,
+wdEvent-vm-def-name,
+(unsigned int)time(NULL));
+
+qemuDriverLock(qemu_driver);
+virDomainObjLock(wdEvent-vm);
+
+if (qemuDomainObjBeginJobWithDriver(qemu_driver, wdEvent-vm)
0)
+break;
+
+if (!virDomainObjIsActive(wdEvent-vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+%s, _(domain is not running));
+break;
+}
+
+ret = doCoreDump(qemu_driver,
+ wdEvent-vm,
+ dumpfile,
+ getCompressionType(qemu_driver));
+if (ret 0)
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+%s, _(Dump failed));
+
+qemuDomainObjEnterMonitorWithDriver(qemu_driver, wdEvent-vm);
+ret = qemuMonitorStartCPUs(priv-mon, NULL);
+qemuDomainObjExitMonitorWithDriver(qemu_driver, wdEvent-vm);
+
+if (ret 0)
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+%s, _(Resuming after dump failed));
+
+ignore_value(qemuDomainObjEndJob(wdEvent-vm));
+
+virDomainObjUnlock(wdEvent-vm);
It isn't safe to ignore the qemuDomainObjEndJob return value,
because if the return value is 0, then the VM object has been
free()d. So you need todo
if (qemuDomainObjEndJob(wdEvent-vm) 0)
virDomainObjUnlock(wdEvent-vm);
+qemuDriverUnlock(qemu_driver);
+
+VIR_FREE(dumpfile);
+}
+break;
+}
+
+VIR_FREE(wdEvent);
+}
I'd prefer it if the 'qemu_driver' was passed in via the 'void *opaque'
parameter. Although it is available via the global variable, we aim to
avoid using that in the driver code, except for the global startup/shutdown
methods.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 2/4] Add a new function doCoreDump
On Thu, Dec 02, 2010 at 03:30:03PM +0800, Hu Tao wrote: This patch prepares for the next patch. --- src/qemu/qemu_driver.c | 132 +++- 1 files changed, 74 insertions(+), 58 deletions(-) ACK, looks fine to me. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 4/4] Using threadpool API to manage qemud worker
On Thu, Dec 02, 2010 at 03:30:23PM +0800, Hu Tao wrote:
---
daemon/libvirtd.c | 172
+
daemon/libvirtd.h |4 +
2 files changed, 33 insertions(+), 143 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 791b3dc..dbd050a 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -67,6 +67,7 @@
#include stream.h
#include hooks.h
#include virtaudit.h
+#include threadpool.h
#ifdef HAVE_AVAHI
# include mdns.h
#endif
@@ -248,7 +249,6 @@ static void sig_handler(int sig, siginfo_t * siginfo,
static void qemudDispatchClientEvent(int watch, int fd, int events, void
*opaque);
static void qemudDispatchServerEvent(int watch, int fd, int events, void
*opaque);
-static int qemudStartWorker(struct qemud_server *server, struct qemud_worker
*worker);
void
qemudClientMessageQueuePush(struct qemud_client_message **queue,
@@ -1383,6 +1383,7 @@ static int qemudDispatchServer(struct qemud_server
*server, struct qemud_socket
client-auth = sock-auth;
client-addr = addr;
client-addrstr = addrstr;
+client-server = server;
addrstr = NULL;
This shouldn't be needed, as 'server' shoudl be passed into
the worker function via the 'void *opaque' parameter.
for (i = 0 ; i VIR_DOMAIN_EVENT_ID_LAST ; i++) {
@@ -1458,19 +1459,6 @@ static int qemudDispatchServer(struct qemud_server
*server, struct qemud_socket
server-clients[server-nclients++] = client;
-if (server-nclients server-nactiveworkers
-server-nactiveworkers server-nworkers) {
-for (i = 0 ; i server-nworkers ; i++) {
-if (!server-workers[i].hasThread) {
-if (qemudStartWorker(server, server-workers[i]) 0)
-return -1;
-server-nactiveworkers++;
-break;
-}
-}
-}
-
-
return 0;
error:
@@ -1534,100 +1522,27 @@ void qemudDispatchClientFailure(struct qemud_client
*client) {
VIR_FREE(client-addrstr);
}
-
-/* Caller must hold server lock */
-static struct qemud_client *qemudPendingJob(struct qemud_server *server)
+static void qemudWorker(void *data, void *opaque ATTRIBUTE_UNUSED)
{
-int i;
-for (i = 0 ; i server-nclients ; i++) {
-virMutexLock(server-clients[i]-lock);
-if (server-clients[i]-dx) {
-/* Delibrately don't unlock client - caller wants the lock */
-return server-clients[i];
-}
-virMutexUnlock(server-clients[i]-lock);
-}
-return NULL;
-}
+struct qemud_client *client = data;
+struct qemud_client_message *msg;
-static void *qemudWorker(void *data)
-{
-struct qemud_worker *worker = data;
-struct qemud_server *server = worker-server;
+virMutexLock(client-lock);
It is neccessary to hold the lock on 'server' before obtaining a
lock on 'client'. The server lock can be released again immediately
if no longer needed.
-while (1) {
-struct qemud_client *client = NULL;
-struct qemud_client_message *msg;
+/* Remove our message from dispatch queue while we use it */
+msg = qemudClientMessageQueueServe(client-dx);
-virMutexLock(server-lock);
-while ((client = qemudPendingJob(server)) == NULL) {
-if (worker-quitRequest ||
-virCondWait(server-job, server-lock) 0) {
-virMutexUnlock(server-lock);
-return NULL;
-}
-}
-if (worker-quitRequest) {
-virMutexUnlock(client-lock);
-virMutexUnlock(server-lock);
-return NULL;
-}
-worker-processingCall = 1;
-virMutexUnlock(server-lock);
-
-/* We own a locked client now... */
-client-refs++;
-
-/* Remove our message from dispatch queue while we use it */
-msg = qemudClientMessageQueueServe(client-dx);
-
-/* This function drops the lock during dispatch,
- * and re-acquires it before returning */
-if (remoteDispatchClientRequest (server, client, msg) 0) {
-VIR_FREE(msg);
-qemudDispatchClientFailure(client);
-client-refs--;
-virMutexUnlock(client-lock);
-continue;
-}
-
-client-refs--;
-virMutexUnlock(client-lock);
-
-virMutexLock(server-lock);
-worker-processingCall = 0;
-virMutexUnlock(server-lock);
-}
-}
-
-static int qemudStartWorker(struct qemud_server *server,
-struct qemud_worker *worker) {
-pthread_attr_t attr;
-pthread_attr_init(attr);
-/* We want to join workers, so don't detach them */
-/*pthread_attr_setdetachstate(attr, 1);*/
-
-if (worker-hasThread)
-return -1;
-
-worker-server = server;
-worker-hasThread = 1;
-
Re: [libvirt] GNU awk vs BSD awk?
On Thu, Dec 02, 2010 at 03:08:28PM +1100, Justin Clift wrote: Hi Eric, One of the OSX Homebrew guys, CC'd, has asked (casually) whether our dependency on GNU awk is very deeply ingrained, or if we could be convinced to allow usage of other make's instead (ie BSD). Figured I'd ask you directly, as you'd probably best know whether it's an easy thing to address or not? What problems are hit with BSD awk ? If it is a minor change then I'm not against it, but in general I think we should only focus on building with the GNU toolchain, since that is easily made available on any platform we support. Even if they already have similar tools (awk/make/etc), the GNU stuff can sit alongside native versions typically offers far more functionality. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] smbios fixes
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: In testing smbios mode='host'/, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the 'uuid' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/8] Fix memory leak in logging setup
On Wed, Dec 01, 2010 at 12:21:51PM -0700, Eric Blake wrote:
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
The logging setup requires const char * strings, but the
virLogSetFromEnv() strdup's the env variables, thus causing
a memory leak
* src/util/logging.c: Avoid strdup'ing env variables
---
src/util/logging.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/logging.c b/src/util/logging.c
index d65dec0..83cc358 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -980,8 +980,8 @@ void virLogSetFromEnv(void) {
virLogParseDefaultPriority(debugEnv);
debugEnv = getenv(LIBVIRT_LOG_FILTERS);
if (debugEnv *debugEnv)
-virLogParseFilters(strdup(debugEnv));
+virLogParseFilters(debugEnv);
ACK; independently useful to push now, even if the rest of your series
is not ready for prime time.
Yep, pushed that
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] Tweak daemon event debug to include errno
On Wed, Dec 01, 2010 at 12:22:53PM -0700, Eric Blake wrote:
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
* daemon/event.c: Include errno in debug info upon poll() failure
---
daemon/event.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/daemon/event.c b/daemon/event.c
index a983b35..0920748 100644
--- a/daemon/event.c
+++ b/daemon/event.c
@@ -576,13 +576,14 @@ int virEventRunOnce(void) {
retry:
EVENT_DEBUG(Poll on %d handles %p timeout %d, nfds, fds, timeout);
ret = poll(fds, nfds, timeout);
-EVENT_DEBUG(Poll got %d event, ret);
if (ret 0) {
+EVENT_DEBUG(Poll got error event %d, errno);
if (errno == EINTR) {
goto retry;
}
goto error_unlocked;
}
+EVENT_DEBUG(Poll got %d event(s), ret);
ACK; independently useful, if you want to push now.
Pushed too
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] Introduce generic objects for building XDR RPC servers/clients
On Wed, Dec 01, 2010 at 12:30:25PM -0700, Eric Blake wrote: On 12/01/2010 10:26 AM, Daniel P. Berrange wrote: Introduces a set of generic objects which are to be used in building RPC servers/clients based on XDR. - virNetMessageHeader - standardize the XDR format for any RPC program. Copied from remote protocol for back compat - virNetMessage - Provides a buffer for (de-)serializing messages, and a copy of the decoded virNetMessageHeader. Provides APIs for encoding/decoding message headers and payloads, thus isolating all the XDR api calls in one file. Callers no longer need to use XDR themselves. - virNetSocket - a wrapper around a socket file descriptor, to simplify creation of new sockets, both for clients and services. Encapsulates all the hairy getaddrinfo code and sockaddr manipulation. Will eventually include transparent support for TLS and SASL encoding of data - virNetTLSContext - encapsulates the credentials required to setup TLS sessions. eg the set of x509 certificates and keys, optional DH parameters and x509 DName whitelist Provides APIs for easily validating certificates from a TLS session - virNetTLSSession - encapsulates the TLS session handling, so that callers no longer have a direct dependancy on gnutls. This will facilitate adding alternate TLS impls. Makes the read/write TLS functions work with same semantics as the native socket read/write functions. ie they set errno, instead of a gnutls specific error code. Is it worth introducing these in separate patches, instead of all in one go? At any rate, this is big enough that I haven't reviewed it in detail yet, but the concept of factoring out the common code seems nice. Yep, I could probably split this into 3 patches, without too much pain Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] man pages: update the description for the virsh help command
Now includes information on keyword usage, and provides examples. --- tools/virsh.pod | 37 ++--- 1 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index c97786a..66654a7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -115,10 +115,41 @@ The following commands are generic i.e. not specific to a domain. =over 4 -=item Bhelp optional Icommand +=item Bhelp optional I--command Icommand | Igroup-keyword -This prints a small synopsis about all commands available for Bvirsh -Bhelp Icommand will print out a detailed help message on that command. +This lists each of the virsh commands. When used without options, all +commands are listed, one per line, grouped into related categories, +displaying the keyword for each group. + +To display only commands for a specific group, give the keyword for that +group as an option. For example: + + virsh # help host + + Host and Hypervisor (help keyword 'host'): + capabilities capabilities + connect(re)connect to hypervisor + freecell NUMA free memory + hostname print the hypervisor hostname + qemu-monitor-command Qemu Monitor Command + uriprint the hypervisor canonical URI + +To display detailed information for a specific command, give its name as the +option instead. For example: + + virsh # help list + NAME + list - list domains + + SYNOPSIS + list [--inactive] [--all] + + DESCRIPTION + Returns list of domains. + + OPTIONS + --inactive list inactive domains + --alllist inactive active domains =item Bquit, Bexit -- 1.7.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VolumeCreateXML XML description for ESX
Great ! it did pass the error of directory/file thank you for the support
now i have another problem of creating the VMDK image with this XML
volumenametest_vm/test_vm.vmdk/nameallocation0/allocationcapacity
unit='G'2/capacitytargetformat type='vmdk'//target/volume
and error
libvir: ESX error : internal error Could not create volume
Traceback (most recent call last):
File stdin, line 1, in module
File /usr/lib64/python2.7/site-packages/libvirt.py, line 1116, in
createXML
if ret is None:raise libvirtError('virStorageVolCreateXML() failed',
pool=self)
libvirt.libvirtError: internal error Could not create volume
may be it is an ESX server side problem or i am still missing or messing up
something ?
Regards,
sherif
On Thu, Dec 2, 2010 at 2:37 PM, Matthias Bolte
matthias.bo...@googlemail.com wrote:
This XML snippet should work
volume
nametest_vm/test_vm.vmdk/name
allocation0/allocation
capacity unit='G'2/capacity
/volume
The error message says that the volume name doesn't have the expected
format directory/file. The might be misleading here, they don't
refer to XML elements.
I can probably relax this and allow files in the datastore root. The
problem with a .vmdk file in the datastore root is that ESX doesn't
allow a virtual machine to be registered (or defined in libvirt terms)
in the datastore root. The typical layout is to have a subdirectory
per virtual machine.
Matthias
2010/12/2 Sherif Nagy sherif.n...@gmail.com:
The XML i am trying to use is
volumenametest_vm.vmdk/namekey/keyallocation0/allocationcapacity
unit='G'2/capacity/volume
or
volumenametest_vm.vmdk/namedirectorytest_vmfiletest_vm.vmdk/file/directorykey/keyallocation0/allocationcapacity
unit='G'2/capacity/volume
and if i added directory and file directive i am still getting the same
error, i am not sure if i am doing something wrong or it is a bug , can
someone please advice me what is the correct XML structure for creating
volume using the ESX driver ?
Thank You
Regards,
Sherif
On Thu, Dec 2, 2010 at 2:04 PM, Justin Clift jcl...@redhat.com wrote:
Matthias, sounds like there's a bug or we need to update the docs?
On 02/12/2010, at 11:02 PM, Sherif Nagy wrote:
yes i am using same template like KVM but it is not working, seems ESX
driver has a different XML structure since it is using datatstore
/dir/filename.vdk structure.
so any idea what is the XML structure for creating the XML for volumes
using the ESX driver?
On Thu, Dec 2, 2010 at 12:48 PM, Justin Clift jcl...@redhat.com
wrote:
On 02/12/2010, at 8:18 PM, Sherif Nagy wrote:
Hello,
I am using libvirt 0.8.6 python bindings can someone point me to
where i find the XML description of creating volumes using
storagecolumecreateXML function ? i am getting libvir: ESX error :
internal
error Volume name 'name.vmdk' doesn't have expected format
'directory/file'
Hi Sherif,
In theory, it's probably supposed to work with the standard storage
and pool XML format documented here:
http://libvirt.org/formatstorage.html
But, it sounds like in practise that's not working for you. Is that
the
case?
Regards and best wishes,
Justin Clift
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Looking for Hypervisor Vulerability Example
On Tue, Nov 30, 2010 at 01:08:12PM -0800, Shi Jin wrote: Hi there, I am researching on virtualization security and particularly on sVirt. From this sVirt presentation[1] and this RHEL-6 documentation on sVirt [2], I read: If there is a security flaw in the hypervisor that can be exploited by a guest instance, this guest may be able to not only attack the host, but also other guests running on that host. This is not theoretical; attacks already exist on hypervisors. These attacks can extend beyond the guest instance and could expose other guests to attack. I am very interested to know about the exact attacks: which version of hypervisor on which OS, how was the exploit used and how it affected the systems. James Morris' presentation is referring to this published demonstration of exploiting Xen a few years ago http://www.securityfocus.com/archive/1/497376 http://invisiblethingslab.com/resources/misc08/xenfb-adventures-10.pdf The key difference sVirt makes is at chapter 3.4 in the paper. In Xen world, there was a single SELinux domain (xend_t) that covered XenD and all the QEMU processes. Since all VMs XenD ran as the same context, any exploited QEMU process in Xen, could access any other guest disks, as well as any host disks. In the KVM + sVirt world, every QEMU process is separated by a dedicated MCS category on its SELinux context. The disks assigned to a guest are labelled with the same MCS category. This means that an exploited QEMU can only access disks which were explicitly assigned to it, and cannot access the host disk devices. This prevents the step in that paper where they overwrite various key files in the host OS root filesystem Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add network disk support
On Fri, Nov 26, 2010 at 05:49:56AM +0900, MORITA Kazutaka wrote:
This patch adds network disk support to libvirt/QEMU. The currently
supported protcols are nbd, rbd, and sheepdog. The XML syntax is like
this:
disk type=network device=disk
driver name=qemu type=raw /
source protocol='rbd|sheepdog|nbd' name=...some image identifier...
host name=mon1.example.org port=6000
host name=mon2.example.org port=6000
host name=mon3.example.org port=6000
/source
target dev=vda bus=virtio /
/disk
This design looks good to me.
Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
---
This patch addresses the discussion on
https://www.redhat.com/archives/libvir-list/2010-November/msg00759.html
Josh mentioned that the monitor hostnames of RBD can be set through
the environment variables, but I couldn't find any documentations
about it, so the monitors are not set in this patch. I hope someone
who is familiar with RBD implements it.
@@ -1620,6 +1629,30 @@ virDomainDiskDefParseXML(virCapsPtr caps,
case VIR_DOMAIN_DISK_TYPE_DIR:
source = virXMLPropString(cur, dir);
break;
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+protocol = virXMLPropString(cur, protocol);
+if (protocol == NULL) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ %s, _(missing protocol
type));
+break;
+}
+def-protocol =
virDomainDiskProtocolTypeFromString(protocol);
Should check for def-protocal 0 raise an error, which
would indicate that 'protocol' was not one of the expected
values.
+source = virXMLPropString(cur, name);
+host = cur-children;
+while (host != NULL) {
+if (host-type == XML_ELEMENT_NODE
+xmlStrEqual(host-name, BAD_CAST host)) {
+if (VIR_REALLOC_N(hosts, def-nhosts + 1) 0) {
+virReportOOMError();
+goto error;
+}
+hosts[def-nhosts].name = virXMLPropString(host,
name);
+hosts[def-nhosts].port = virXMLPropString(host,
port);
+def-nhosts++;
Ought to check for NULLs returned by virXMLPropString here
I think.
+}
+host = host-next;
+}
+break;
default:
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
_(unexpected disk type %s),
@@ -1685,7 +1718,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
/* Only CDROM and Floppy devices are allowed missing source path
* to indicate no media present */
-if (source == NULL
+if (source == NULL hosts == NULL
def-device != VIR_DOMAIN_DISK_DEVICE_CDROM
def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
virDomainReportError(VIR_ERR_NO_SOURCE,
@@ -1791,6 +1824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
source = NULL;
def-dst = target;
target = NULL;
+def-hosts = hosts;
+hosts = NULL;
def-driverName = driverName;
driverName = NULL;
def-driverType = driverType;
@@ -1819,6 +1854,8 @@ cleanup:
VIR_FREE(type);
VIR_FREE(target);
VIR_FREE(source);
+VIR_FREE(hosts);
I think you need to free the fields inside 'hosts' too, otherwise
we'd leak memory for the port + host strings in the error path.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 35caccc..63abd75 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2714,7 +2714,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
break;
}
-if (disk-src) {
+if (disk-src || disk-nhosts 0) {
If you check for nhosts 0 here
if (disk-type == VIR_DOMAIN_DISK_TYPE_DIR) {
/* QEMU only supports magic FAT format for now */
if (disk-driverType
@@ -2733,6 +2733,24 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
virBufferVSprintf(opt, file=fat:floppy:%s,, disk-src);
else
virBufferVSprintf(opt, file=fat:%s,, disk-src);
+} else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
+switch (disk-protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+virBufferVSprintf(opt, file=nbd:%s:%s,,
+ disk-hosts-name, disk-hosts-port);
+break;
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+virBufferVSprintf(opt, file=rbd:%s,, disk-src);
+break;
+case
[libvirt] [PATCH] Create file in virFileWriteStr() if it doesn't exist
This patch adds a virFileWriteStrEx() function with a
third parameter to set the created file permissions.
virFileWriteStr() calls this new function with a
default value for the mode parameter.
---
src/util/util.c | 11 ---
src/util/util.h |1 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index a2582aa..82ca9b3 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1123,14 +1123,19 @@ int virFileReadAll(const char *path, int maxlen, char
**buf)
return len;
}
-/* Truncate @path and write @str to it.
+int virFileWriteStr(const char *path, const char *str)
+{
+return virFileWriteStrEx(path, str, S_IRUSR|S_IWUSR);
+}
+
+/* Truncate or create @path and write @str to it.
Return 0 for success, nonzero for failure.
Be careful to preserve any errno value upon failure. */
-int virFileWriteStr(const char *path, const char *str)
+int virFileWriteStrEx(const char *path, const char *str, mode_t mode)
{
int fd;
-if ((fd = open(path, O_WRONLY|O_TRUNC)) == -1)
+if ((fd = open(path, O_WRONLY|O_TRUNC|O_CREAT, mode)) == -1)
return -1;
if (safewrite(fd, str, strlen(str)) 0) {
diff --git a/src/util/util.h b/src/util/util.h
index a240d87..18ef693 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -96,6 +96,7 @@ int virFileReadLimFD(int fd, int maxlen, char **buf)
ATTRIBUTE_RETURN_CHECK;
int virFileReadAll(const char *path, int maxlen, char **buf)
ATTRIBUTE_RETURN_CHECK;
+int virFileWriteStrEx(const char *path, const char *str, mode_t mode)
ATTRIBUTE_RETURN_CHECK;
int virFileWriteStr(const char *path, const char *str) ATTRIBUTE_RETURN_CHECK;
int virFileMatchesNameSuffix(const char *file,
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] cgroups net_cls controller implementation
This a basic implemantation to support the net_cls feature of
cgroups. It adds the setting of a net_cls.classid value to the
existing cgroups setup in the qemu driver.
The classid is specified in the qemu.conf file.
This enables the use of the tc utility to manage traffic from/to
vitual machines
based on the setting combination of classid and network interface.
Signed-off-by: D.Herrendoerfer d.herrendoerfer [at] herrendoerfer
[dot] name
src/libvirt_private.syms |1 +
src/qemu/qemu.conf |6 +-
src/qemu/qemu_conf.c |7 ++-
src/qemu/qemu_conf.h |1 +
src/qemu/qemu_driver.c | 12
src/util/cgroup.c| 18 +-
src/util/cgroup.h|3 +++
7 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f251c94..771911e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -80,6 +80,7 @@ virCgroupSetFreezerState;
virCgroupSetMemory;
virCgroupSetMemoryHardLimit;
virCgroupSetMemorySoftLimit;
+virCgroupSetNetworkClassID;
virCgroupSetSwapHardLimit;
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f4f965e..591d8dc 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -157,7 +157,7 @@
# can be mounted in different locations. libvirt will detect
# where they are located.
#
-# cgroup_controllers = [ cpu, devices, memory ]
+# cgroup_controllers = [ cpu, devices, memory, net_cls ]
# This is the basic set of devices allowed / required by
# all virtual machines.
@@ -175,6 +175,10 @@
#/dev/rtc, /dev/hpet, /dev/net/tun,
#]
+# This is the default classid that will be assigned
+# to all virtual machines.
+# cgroup_net_cls_classid = 4096
+
# The default format for Qemu/KVM guest save images is raw; that is,
the
# memory from the domain is dumped out directly to a file. If you
have
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7cd0603..46ac040 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -326,7 +326,8 @@ int qemudLoadDriverConfig(struct qemud_driver
*driver,
driver-cgroupControllers =
(1 VIR_CGROUP_CONTROLLER_CPU) |
(1 VIR_CGROUP_CONTROLLER_DEVICES) |
-(1 VIR_CGROUP_CONTROLLER_MEMORY);
+(1 VIR_CGROUP_CONTROLLER_MEMORY) |
+(1 VIR_CGROUP_CONTROLLER_NETWORK);
}
for (i = 0 ; i VIR_CGROUP_CONTROLLER_LAST ; i++) {
if (driver-cgroupControllers (1 i)) {
@@ -364,6 +365,10 @@ int qemudLoadDriverConfig(struct qemud_driver
*driver,
driver-cgroupDeviceACL[i] = NULL;
}
+p = virConfGetValue (conf, cgroup_net_cls_classid);
+CHECK_TYPE (cgroup_net_cls_classid, VIR_CONF_LONG);
+if (p) driver-cgroupNetClsClassid = p-l;
+
p = virConfGetValue (conf, save_image_format);
CHECK_TYPE (save_image_format, VIR_CONF_STRING);
if (p p-str) {
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index aba64d6..961c6cd 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -119,6 +119,7 @@ struct qemud_driver {
virCgroupPtr cgroup;
int cgroupControllers;
char **cgroupDeviceACL;
+int cgroupNetClsClassid;
virDomainObjList domains;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1a7c1ad..42448b5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -186,6 +186,8 @@ static int qemudVMFiltersInstantiate(virConnectPtr
conn,
static struct qemud_driver *qemu_driver = NULL;
+#include interface.h
+
static void *qemuDomainObjPrivateAlloc(void)
{
@@ -3597,6 +3599,16 @@ static int qemuSetupCgroup(struct qemud_driver
*driver,
vm-def-name);
}
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_NETWORK)) {
+
+ if (driver-cgroupNetClsClassid != 0) {
+ rc = virCgroupSetNetworkClassID(cgroup, driver-
cgroupNetClsClassid);
+ if (rc != 0) {
+ VIR_WARN(Cannot set net_cls.classid for: %s,
+ vm-def-name);
+ }
+ }
+}
done:
virCgroupFree(cgroup);
return 0;
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 2758a8f..a2ed0ed 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -37,7 +37,7 @@
VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST,
cpu, cpuacct, cpuset, memory, devices,
- freezer);
+ freezer, net_cls);
struct virCgroupController {
int type;
@@ -851,6 +851,22 @@ int virCgroupForDomain(virCgroupPtr driver
ATTRIBUTE_UNUSED,
#endif
/**
+ * virCgroupSetNetworkClassID:
+ *
+ * @group: The cgroup to change memory for
+ * @classid: The classID number
+ *
+ * Returns: 0 on success
+ */
+int virCgroupSetNetworkClassID(virCgroupPtr group, unsigned long
classid)
+{
+return virCgroupSetValueU64(group,
+VIR_CGROUP_CONTROLLER_NETWORK,
+
[libvirt] [Patch v2] Add VMware Workstation and Player driver
Many changes since v2 based on Matthias review (http://www.redhat.com/archives/libvir-list/2010-November/msg00799.html). popen() calls will be removed when the new virCommand API is available. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/8] util: add virVasprintf
On Tue, Nov 23, 2010 at 04:49:54PM -0700, Eric Blake wrote: * src/util/util.h (virVasprintf): New declaration. * src/util/util.c (virVasprintf): New function. (virAsprintf): Use it. * src/util/virtaudit.c (virAuditSend): Likewise. * src/libvirt_private.syms: Export it. * cfg.mk (sc_prohibit_asprintf): Also prohibit vasprintf. * .x-sc_prohibit_asprintf: Add exemption. --- v2: new patch; makes virCommandAddArgFormat possible in later patch .x-sc_prohibit_asprintf |4 +++- cfg.mk |2 +- src/libvirt_private.syms |1 + src/util/util.c | 21 + src/util/util.h |6 +- src/util/virtaudit.c |2 +- 6 files changed, 28 insertions(+), 8 deletions(-) ACK, this is useful in a few other existing places too. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/8] util: fix saferead type
On Tue, Nov 23, 2010 at 04:49:55PM -0700, Eric Blake wrote:
* src/util/util.c (saferead): Fix return type.
(safewrite): Fix indentation.
---
v2: new patch. Not really essential to the series, so much as a
trivial cleanup I noticed while in the area.
src/util/util.c | 64 --
src/util/util.h |2 +-
2 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 3a27c23..f6050de 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -88,42 +88,44 @@ verify(sizeof(gid_t) = sizeof (unsigned int)
__FUNCTION__, __LINE__, __VA_ARGS__)
/* Like read(), but restarts after EINTR */
-int saferead(int fd, void *buf, size_t count)
-{
-size_t nread = 0;
-while (count 0) {
-ssize_t r = read(fd, buf, count);
-if (r 0 errno == EINTR)
-continue;
-if (r 0)
-return r;
-if (r == 0)
-return nread;
-buf = (char *)buf + r;
-count -= r;
-nread += r;
-}
-return nread;
+ssize_t
+saferead(int fd, void *buf, size_t count)
+{
+size_t nread = 0;
+while (count 0) {
+ssize_t r = read(fd, buf, count);
+if (r 0 errno == EINTR)
+continue;
+if (r 0)
+return r;
+if (r == 0)
+return nread;
+buf = (char *)buf + r;
+count -= r;
+nread += r;
+}
+return nread;
}
/* Like write(), but restarts after EINTR */
-ssize_t safewrite(int fd, const void *buf, size_t count)
-{
-size_t nwritten = 0;
-while (count 0) {
-ssize_t r = write(fd, buf, count);
-
-if (r 0 errno == EINTR)
-continue;
-if (r 0)
-return r;
-if (r == 0)
-return nwritten;
-buf = (const char *)buf + r;
-count -= r;
-nwritten += r;
-}
-return nwritten;
+ssize_t
+safewrite(int fd, const void *buf, size_t count)
+{
+size_t nwritten = 0;
+while (count 0) {
+ssize_t r = write(fd, buf, count);
+
+if (r 0 errno == EINTR)
+continue;
+if (r 0)
+return r;
+if (r == 0)
+return nwritten;
+buf = (const char *)buf + r;
+count -= r;
+nwritten += r;
+}
+return nwritten;
}
#ifdef HAVE_POSIX_FALLOCATE
diff --git a/src/util/util.h b/src/util/util.h
index edbf01e..deaf8bb 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -37,7 +37,7 @@
# define MIN(a, b) ((a) (b) ? (a) : (b))
# endif
-int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
+ssize_t saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
ssize_t safewrite(int fd, const void *buf, size_t count)
ATTRIBUTE_RETURN_CHECK;
int safezero(int fd, int flags, off_t offset, off_t len)
ACK
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/8] Introduce new APIs for spawning processes
On Tue, Nov 23, 2010 at 04:49:56PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com This introduces a new set of APIs in src/util/command.h to use for invoking commands. This is intended to replace all current usage of virRun and virExec variants, with a more flexible and less error prone API. * src/util/command.c: New file. * src/util/command.h: New header. * src/Makefile.am (UTIL_SOURCES): Build it. * src/libvirt_private.syms: Export symbols internally. * tests/commandtest.c: New test. * tests/Makefile.am (check_PROGRAMS): Run it. * tests/commandhelper.c: Auxiliary program. * tests/commanddata/test2.log - test15.log: New expected outputs. * cfg.mk (useless_free_options): Add virCommandFree. * po/POTFILES.in: New translation. * .x-sc_avoid_write: Add exemption. * tests/.gitignore: Ignore new built file. ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/8] virCommand: docs for usage of new command APIs
On Tue, Nov 23, 2010 at 04:49:57PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com * docs/internals/command.html.in: New file. * docs/Makefile.am: Build new docs. * docs/subsite.xsl: New glue file. * docs/internals.html.in, docs/sitemap.html.in: Update glue. --- v2: document commands added in v2. docs/Makefile.am | 11 +- docs/internals.html.in |9 + docs/internals/command.html.in | 550 docs/sitemap.html.in |4 + docs/subsite.xsl | 25 ++ 5 files changed, 598 insertions(+), 1 deletions(-) create mode 100644 docs/internals/command.html.in create mode 100644 docs/subsite.xsl ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 7/8] uml: convert to virCommand
On Tue, Nov 23, 2010 at 04:50:00PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com * src/uml/uml_conf.c (umlBuildCommandLineChr) (umlBuildCommandLine): Rewrite with virCommand. * src/uml/uml_conf.h (umlBuildCommandLine): Update signature. * src/uml/uml_driver.c (umlStartVMDaemon): Adjust caller. --- v2: new patch (well, Dan started it in May, but this is the first time I've cleaned it up enough to be worth posting) ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 5/8] Port hooks and iptables code to new command execution APIs
On Tue, Nov 23, 2010 at 04:49:58PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com This proof of concept shows how two existing uses of virExec and virRun can be ported to the new virCommand APIs, and how much simpler the code becomes ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
Il giorno gio, 02/12/2010 alle 10.58 +, Daniel P. Berrange ha scritto: I think this is really viable, because it implies we need another week prior to creating the pre-release where we do what we currently do with pre-release stabalization. Note that I said “one week or less”. - A official list of supported platforms / OS combinations - Run a test build on each combination before release - Actually follow the 'bug fixes' only rule leading upto release no matter how simple the new feature might appear. And let me guess that the supported platforms are going to be mostly RedHatco.? Why not warning the rest of the packagers, so that each run their tests? -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 6/8] qemu: convert to virCommand
On Tue, Nov 23, 2010 at 04:49:59PM -0700, Eric Blake wrote: * src/qemu/qemu_conf.c (qemudExtractVersionInfo): Check for file before executing it here, rather than in callers. (qemudBuildCommandLine): Rewrite with virCommand. * src/qemu/qemu_conf.h (qemudBuildCommandLine): Update signature. * src/qemu/qemu_driver.c (qemuAssignPCIAddresses) (qemudStartVMDaemon, qemuDomainXMLToNative): Adjust callers. --- v2: new patch, taking most of the ideas from https://www.redhat.com/archives/libvir-list/2010-November/msg00979.html but making it better by using virCommand improvements ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 8/8] Remove bogus includes
On Tue, Nov 23, 2010 at 04:50:01PM -0700, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com --- v2: rearrange to later in the series; no other change. Passes for me with macvtap compilation enabled, so I'm not sure if it still suffers from the same problem as the v1 complaint: https://www.redhat.com/archives/libvir-list/2010-November/msg00834.html src/conf/domain_conf.c |1 - src/util/hooks.c |1 - 2 files changed, 0 insertions(+), 2 deletions(-) ACK The problem I hit was actually with removing diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 5dcc9e1..eb4ea8f 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -49,7 +49,6 @@ # include logging.h # include macvtap.h # include interface.h -# include conf/domain_conf.h Because the 'util' directory must never depend on the 'conf' directory. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/8] Introduce new APIs for spawning processes
On Wed, Dec 01, 2010 at 04:39:25PM -0700, Eric Blake wrote: On 11/23/2010 04:49 PM, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com This introduces a new set of APIs in src/util/command.h to use for invoking commands. This is intended to replace all current usage of virRun and virExec variants, with a more flexible and less error prone API. +/* + * Call after adding all arguments and environment settings, but before + * Run/RunAsync, to immediately output the environment and arguments of + * cmd to logfd. If virCommandRun cannot succeed (because of an + * out-of-memory condition while building cmd), nothing will be logged. + */ +void virCommandWriteArgLog(virCommandPtr cmd, + int logfd); + +/* + * Call after adding all arguments and environment settings, but before + * Run/RunAsync, to return a string representation of the environment and + * arguments of cmd. If virCommandRun cannot succeed (because of an + * out-of-memory condition while building cmd), NULL will be returned. + * Caller is responsible for freeing the resulting string. + */ +char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; Bummer. Just realized that these functions should probably be modified to take another parameter that controls whether the output should be quoted for shell use. I don't think this is particularly important given the usage made of these API. It is just a nice to have addition, which shouldn't delay the patchset. Dnaiel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/8] Introduce new APIs for spawning processes
On Wed, Dec 01, 2010 at 10:42:40PM +0100, Matthias Bolte wrote:
2010/11/24 Eric Blake ebl...@redhat.com:
+/*
+ * Manage input and output to the child process.
+ */
+static int
+virCommandProcessIO(virCommandPtr cmd)
+{
+ int infd = -1, outfd = -1, errfd = -1;
+ size_t inlen = 0, outlen = 0, errlen = 0;
+ size_t inoff = 0;
+
+ /* With an input buffer, feed data to child
+ * via pipe */
+ if (cmd-inbuf) {
+ inlen = strlen(cmd-inbuf);
+ infd = cmd-inpipe;
+ }
+
+ /* With out/err buffer, the outfd/errfd
+ * have been filled with an FD for us */
+ if (cmd-outbuf)
+ outfd = cmd-outfd;
+ if (cmd-errbuf)
+ errfd = cmd-errfd;
+
+/*
+ * Run the command and wait for completion.
+ * Returns -1 on any error executing the
+ * command. Returns 0 if the command executed,
+ * with the exit status set
+ */
+int
+virCommandRun(virCommandPtr cmd, int *exitstatus)
+{
+ int ret = 0;
+ char *outbuf = NULL;
+ char *errbuf = NULL;
+ int infd[2];
+
+ if (!cmd || cmd-has_error == -1) {
+ virCommandError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(invalid use of command API));
+ return -1;
+ }
+ if (cmd-has_error == ENOMEM) {
+ virReportOOMError();
+ return -1;
+ }
+
+ /* If we have an input buffer, we need
+ * a pipe to feed the data to the child */
+ if (cmd-inbuf) {
+ if (pipe(infd) 0) {
+ virReportSystemError(errno, %s,
+ _(unable to open pipe));
+ return -1;
+ }
+ cmd-infd = infd[0];
+ cmd-inpipe = infd[1];
+ }
+
+ if (virCommandRunAsync(cmd, NULL) 0) {
+ if (cmd-inbuf) {
+ int tmpfd = infd[0];
+ if (VIR_CLOSE(infd[0]) 0)
+ VIR_DEBUG(ignoring failed close on fd %d, tmpfd);
+ tmpfd = infd[1];
+ if (VIR_CLOSE(infd[1]) 0)
+ VIR_DEBUG(ignoring failed close on fd %d, tmpfd);
+ }
+ return -1;
+ }
+
+ /* If caller hasn't requested capture of
+ * stdout/err, then capture it ourselves
+ * so we can log it
+ */
+ if (!cmd-outbuf
+ !cmd-outfdptr) {
+ cmd-outfd = -1;
+ cmd-outfdptr = cmd-outfd;
+ cmd-outbuf = outbuf;
+ }
+ if (!cmd-errbuf
+ !cmd-errfdptr) {
+ cmd-errfd = -1;
+ cmd-errfdptr = cmd-errfd;
+ cmd-errbuf = errbuf;
+ }
+
+ if (cmd-inbuf || cmd-outbuf || cmd-errbuf)
+ ret = virCommandProcessIO(cmd);
In virCommandProcessIO you say that cmd-{out|err}fd is a valid FD
(created in virExecWithHook called by virCommandRunAsync) when
cmd-{out|err}buf is not NULL. As far as I can tell from the code that
is true when the caller had requested to capture stdout and stderr.
But in case that caller didn't do this then you setup buffers to
capture stdout and stderr for logging. In that case
virCommandProcessIO will be called with cmd-{out|err}buf being
non-NULL but cmd-{out|err}fd being invalid as you explicitly set them
to -1 in the two if blocks before the call to virCommandProcessIO.
Those two blocks setting errfd/outfd to -1 are not executed
when outbuf/errbuf are non-NULL, so errfd/outfd are still
pointing to the pipe() FDs when virCommandProcesIO is run.
diff --git a/tests/commandtest.c b/tests/commandtest.c
new file mode 100644
index 000..46d6ae5
--- /dev/null
+++ b/tests/commandtest.c
+
+#ifndef __linux__
What's Linux specific in this test? Shouldn't the command API and this
test be working on all systems that support fork/exec?
It should have been #ifndef WIN32 actually.
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/8] Introduce new APIs for spawning processes
2010/12/2 Daniel P. Berrange berra...@redhat.com:
On Wed, Dec 01, 2010 at 10:42:40PM +0100, Matthias Bolte wrote:
2010/11/24 Eric Blake ebl...@redhat.com:
+/*
+ * Manage input and output to the child process.
+ */
+static int
+virCommandProcessIO(virCommandPtr cmd)
+{
+ int infd = -1, outfd = -1, errfd = -1;
+ size_t inlen = 0, outlen = 0, errlen = 0;
+ size_t inoff = 0;
+
+ /* With an input buffer, feed data to child
+ * via pipe */
+ if (cmd-inbuf) {
+ inlen = strlen(cmd-inbuf);
+ infd = cmd-inpipe;
+ }
+
+ /* With out/err buffer, the outfd/errfd
+ * have been filled with an FD for us */
+ if (cmd-outbuf)
+ outfd = cmd-outfd;
+ if (cmd-errbuf)
+ errfd = cmd-errfd;
+
+/*
+ * Run the command and wait for completion.
+ * Returns -1 on any error executing the
+ * command. Returns 0 if the command executed,
+ * with the exit status set
+ */
+int
+virCommandRun(virCommandPtr cmd, int *exitstatus)
+{
+ int ret = 0;
+ char *outbuf = NULL;
+ char *errbuf = NULL;
+ int infd[2];
+
+ if (!cmd || cmd-has_error == -1) {
+ virCommandError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(invalid use of command API));
+ return -1;
+ }
+ if (cmd-has_error == ENOMEM) {
+ virReportOOMError();
+ return -1;
+ }
+
+ /* If we have an input buffer, we need
+ * a pipe to feed the data to the child */
+ if (cmd-inbuf) {
+ if (pipe(infd) 0) {
+ virReportSystemError(errno, %s,
+ _(unable to open pipe));
+ return -1;
+ }
+ cmd-infd = infd[0];
+ cmd-inpipe = infd[1];
+ }
+
+ if (virCommandRunAsync(cmd, NULL) 0) {
+ if (cmd-inbuf) {
+ int tmpfd = infd[0];
+ if (VIR_CLOSE(infd[0]) 0)
+ VIR_DEBUG(ignoring failed close on fd %d, tmpfd);
+ tmpfd = infd[1];
+ if (VIR_CLOSE(infd[1]) 0)
+ VIR_DEBUG(ignoring failed close on fd %d, tmpfd);
+ }
+ return -1;
+ }
+
+ /* If caller hasn't requested capture of
+ * stdout/err, then capture it ourselves
+ * so we can log it
+ */
+ if (!cmd-outbuf
+ !cmd-outfdptr) {
+ cmd-outfd = -1;
+ cmd-outfdptr = cmd-outfd;
+ cmd-outbuf = outbuf;
+ }
+ if (!cmd-errbuf
+ !cmd-errfdptr) {
+ cmd-errfd = -1;
+ cmd-errfdptr = cmd-errfd;
+ cmd-errbuf = errbuf;
+ }
+
+ if (cmd-inbuf || cmd-outbuf || cmd-errbuf)
+ ret = virCommandProcessIO(cmd);
In virCommandProcessIO you say that cmd-{out|err}fd is a valid FD
(created in virExecWithHook called by virCommandRunAsync) when
cmd-{out|err}buf is not NULL. As far as I can tell from the code that
is true when the caller had requested to capture stdout and stderr.
But in case that caller didn't do this then you setup buffers to
capture stdout and stderr for logging. In that case
virCommandProcessIO will be called with cmd-{out|err}buf being
non-NULL but cmd-{out|err}fd being invalid as you explicitly set them
to -1 in the two if blocks before the call to virCommandProcessIO.
Those two blocks setting errfd/outfd to -1 are not executed
when outbuf/errbuf are non-NULL, so errfd/outfd are still
pointing to the pipe() FDs when virCommandProcesIO is run.
Yes, that's true, but that's not the case I'm talking about.
The case I'm talking about is when the user of the command API didn't
set the cmd-outfdptr nor the cmd-outbuf, so both are NULL. In that
case when the chain virCommandRunAsync - virExecWithHook - __virExec
is called __virExec will bind stdout of the child to /dev/null instead
of a pipe.
So when changing cmd-outfdptr and cmd-outbuf to non-NULL values
after the call to virCommandRunAsync this will not result in having a
valid FD assigned to cmd-outfd before the call to
virCommandProcessIO. Therefore, virCommandProcessIO won't capture
stdout and stderr for logging purpose.
I still think that the two if blocks should be moved in front of
virCommandRunAsync to achieve the described behavior, in the case that
the command API user didn't request to capture stdout/stderr.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
On 03/12/2010, at 1:03 AM, Diego Elio Pettenò wrote: - A official list of supported platforms / OS combinations - Run a test build on each combination before release - Actually follow the 'bug fixes' only rule leading upto release no matter how simple the new feature might appear. And let me guess that the supported platforms are going to be mostly RedHatco.? Heh, where did that come from? I kind of hoped you'd noticed we're (Red Hat) putting effort into widening things from being just Red Hat centric. *Very* much interested in widening out our officially supported distro's and platforms. Why not warning the rest of the packagers, so that each run their tests? I actually kind of thinks it's more that our Communication and self discipline hasn't been as good as needed, looking at the result of the last two releases. So, this is our opportunity to identify and fix. With Gentoo, which are the pieces that you reckon are stable enough that we can document them as being known good, and rely on them tested for each new release? Thinking it can't hurt to start getting that info worked out and put on the site properly. ? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] GNU awk vs BSD awk?
On 12/01/2010 09:08 PM, Justin Clift wrote: Hi Eric, One of the OSX Homebrew guys, CC'd, has asked (casually) whether our dependency on GNU awk is very deeply ingrained, or if we could be convinced to allow usage of other make's instead (ie BSD). GNU awk, or GNU make? There's a difference :) With regards to make, libvirt is firmly in the camp of GNU make and nothing else. We use too many extensions specific to gmake that just aren't present in other implementations, and it is too difficult to figure out how to rewrite those in the lowest-common-denominator of POSIX make syntax. (Side note - the Austin Group, which is in charge of updating the POSIX standard, is currently reviewing several proposals to increase the lowest-common-denominator, such as by standardizing something similar to gmake's $(shell) or BSD makes != assignment, but that is a lengthy process, and you can't expect it to make a difference overnight). With regards to awk, libvirt should be able to get by with any POSIX-compliant awk; if you find a counter-example where things are broken while using BSD awk, it should be relatively easy to patch things to get it working again. Note, however, that there are multiple awk flavors out there; oawk (called awk on some systems) is NOT posix-compliant, and is inadequate for libvirt's needs; while mawk, nawk, BSD awk, and gawk should all be interchangeable given a script that sticks only to POSIX features. In fact, autoconf requires a decent awk just for running ./configure, and substitutes $AWK appropriately. I see several uses where libvirt is hard-coded to awk (for example, tools/virt-pki-validate), but most of those are portable even to oawk, because they avoid POSIX features that were not present in older awk; but I have not audited all of libvirt's awk usages. http://www.gnu.org/software/autoconf/manual/autoconf.html#index-g_t_0040command_007bawk_007d-1731 gives a good overview of features to look for when auditing awk scripts for POSIX vs. oawk compliance, as well as some common bugs in almost-POSIX-compliant implementations and how to work around those bugs. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] smbios fixes
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote: On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: In testing smbios mode='host'/, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the 'uuid' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion. I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] man pages: update the description for the virsh help command
On 12/02/2010 06:04 AM, Justin Clift wrote: Now includes information on keyword usage, and provides examples. --- tools/virsh.pod | 37 ++--- 1 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index c97786a..66654a7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -115,10 +115,41 @@ The following commands are generic i.e. not specific to a domain. =over 4 -=item Bhelp optional Icommand +=item Bhelp optional I--command Icommand | Igroup-keyword Hmm, this doesn't quite match 'virsh help help', which if used literally would translate to: =item Bhelp optional Icommand Igroup On the other hand, I'm thinking we implemented the help command slightly wrong by specifying that it takes two optional strings. Really, it only takes one optional string, which is a command-or-group. For instance, with the current code, 'virsh help --group help' lists a command help, rather than a group help, and 'virsh help --command virsh' lists the group help, rather than a command help. Meanwhile, 'virsh help help virsh' is accepted by the parser, but silently ignores the virsh group argument. So I'm thinking we need yet another patch to virsh.c that reduces opts_help to just one VSH_OT_DATA flag name (whether we keep it named --command, or rename it to --command-or-group, is another question, which is also impacted by whether we decide to implement unambiguous prefix parsing like getopt_long). In the meantime, how about we list this line as: =item Bhelp optional Icommand-or-group ACK with that one-line change; the rest of the patch is uncontroversial, and the virsh.c cleanup can be a separate patch. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] GNU awk vs BSD awk?
On 03/12/2010, at 4:29 AM, Eric Blake wrote: On 12/01/2010 09:08 PM, Justin Clift wrote: Hi Eric, One of the OSX Homebrew guys, CC'd, has asked (casually) whether our dependency on GNU awk is very deeply ingrained, or if we could be convinced to allow usage of other make's instead (ie BSD). GNU awk, or GNU make? There's a difference :) Definitely awk. Mike was asking because I had gawk as a specific dependency of the libvirt Homebrew package. It was needed when I first started the packaging for 0.8.5. However, just tested the 0.8.6 release now, and it compiles/works ok with the OSX provided awk. Gawk not installed. Looks like good timing of that question on Mike's part, and I'll submit a new 0.8.6 package that doesn't have the gawk dependency. Thanks Eric. :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] GNU awk vs BSD awk?
On 03/12/2010, at 5:18 AM, Justin Clift wrote: Looks like good timing of that question on Mike's part, and I'll submit a new 0.8.6 package that doesn't have the gawk dependency. Done. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/8] util: fix saferead type
On 12/02/2010 06:58 AM, Daniel P. Berrange wrote: On Tue, Nov 23, 2010 at 04:49:55PM -0700, Eric Blake wrote: * src/util/util.c (saferead): Fix return type. (safewrite): Fix indentation. --- v2: new patch. Not really essential to the series, so much as a trivial cleanup I noticed while in the area. ACK Thanks; I've pushed 1 and 2, and am working on cleaning up the rest of the series (the rebase to latest libvirt.git introduced a couple of conflicts to resolve). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] smbios fixes
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote: On 12/02/2010 05:47 AM, Daniel P. Berrange wrote: On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: In testing smbios mode='host'/, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the 'uuid' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion. As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest. I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid. I am surprised that libvirt still uses -uuid. -- Gleb. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] smbios fixes
On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote: On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote: On 12/02/2010 05:47 AM, Daniel P. Berrange wrote: On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: In testing smbios mode='host'/, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the 'uuid' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion. As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest. I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid. I am surprised that libvirt still uses -uuid. SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] smbios fixes
On Thu, Dec 02, 2010 at 07:56:03PM +, Daniel P. Berrange wrote: On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote: On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote: On 12/02/2010 05:47 AM, Daniel P. Berrange wrote: On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: In testing smbios mode='host'/, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the 'uuid' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion. As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest. I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid. I am surprised that libvirt still uses -uuid. SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong. What non-x86 platforms libvirt supports? With Xen use -uuid or whatever they support. With KVM use only smbios type=1,uuid=$UUID. There is not valid reason that I see to use both. But if you use both of them for some strange reason please make sure you provide the same value for both. -- Gleb. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] smbios fixes
On Thu, Dec 02, 2010 at 10:04:59PM +0200, Gleb Natapov wrote: On Thu, Dec 02, 2010 at 07:56:03PM +, Daniel P. Berrange wrote: On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote: On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote: On 12/02/2010 05:47 AM, Daniel P. Berrange wrote: On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: In testing smbios mode='host'/, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the 'uuid' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion. As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest. I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid. I am surprised that libvirt still uses -uuid. SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong. What non-x86 platforms libvirt supports? With Xen use -uuid or whatever they support. With KVM use only smbios type=1,uuid=$UUID. There is not valid reason that I see to use both. But if you use both of them for some strange reason please make sure you provide the same value for both. libvirt aims to support any and all QEMU target architectures not merely x86. There's no benefit to using -smbios type=1,uuid=$UUID with KVM, over -uuid $UUID. Changing it only for KVM would needlessly complicate the code. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] new preferences requirement
On Wed, Dec 01, 2010 at 10:26:35AM +, Daniel P. Berrange wrote: On Wed, Dec 01, 2010 at 05:35:38PM +0800, Osier Yang wrote: Hi, all We have some new requirements of preferences, I listed which of them I known, and think is useful as follows: 1) for the path of x509 certificate and keys of client The path of x509 certificate and keys of client is hard coded in remote driver. e.g. /* Defaults for PKI directory. */ # define LIBVIRT_PKI_DIR SYSCONFDIR /pki # define LIBVIRT_CACERT LIBVIRT_PKI_DIR /CA/cacert.pem # define LIBVIRT_CLIENTKEY LIBVIRT_PKI_DIR /libvirt/private /clientkey.pem # define LIBVIRT_CLIENTCERT LIBVIRT_PKI_DIR /libvirt/clientcert.pem We can't assume one set of certs/keys is suitable for all URIs, so making this a preference setting doesn't help. There needs to be a parameter in the URI to specify a cert/key name to override the defaults on a per-connection basis As much as I disliked adding long ugly filenames to the URI, I do not see any way around it now. A single client application may need to open two connections with different cert/key pairs, so a single client.conf or environment variables won't cut it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/8] Introduce new APIs for spawning processes
On 12/01/2010 02:42 PM, Matthias Bolte wrote:
+++ b/cfg.mk
@@ -77,6 +77,7 @@ useless_free_options =\
--name=virCapabilitiesFreeHostNUMACell \
--name=virCapabilitiesFreeMachines \
--name=virCgroupFree \
+ --name=virCommandFree\
--name=virConfFreeList \
--name=virConfFreeValue \
--name=virDomainChrDefFree \
You should also add virCommandError to the msg_gen_function list.
Good catch, and done.
+void
+virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
+{
+if (!cmd || cmd-has_error)
+return;
+
+if (cmd-outfd != -1) {
To detect double assignment properly you need to check for this I think:
if (cmd-outfd != -1 || cmd-outfdptr || cmd-outbuf) {
Almost. There are really only two functions that affect stdout settings
before a command (set fd assigns outfdptr, set buffer assigns outbuf and
outfdptr), so checking cmd-outfdptr for NULL is sufficient to check for
no duplicate call.
+void
+virCommandSetInputFD(virCommandPtr cmd, int infd)
+{
+if (!cmd || cmd-has_error)
+return;
+
+if (infd 0 || cmd-inbuf) {
I think you meant to check here for this instead:
if (cmd-infd != -1 || cmd-inbuf) {
Hmm; actually, there's two issues. One of validating that input is set
at most once (cmd-infd != -1 || cmd-inbuf), and one of validating that
the incoming argument is valid (infd 0), worth two separate messages.
Thanks for forcing me to think about this more.
+void
+virCommandWriteArgLog(virCommandPtr cmd, int logfd)
+{
+int ioError = 0;
+size_t i;
+
+/* Any errors will be reported later by virCommandRun, which means
+ * no command will be run, so there is nothing to log. */
+if (!cmd || cmd-has_error)
+return;
+
+for (i = 0 ; i cmd-nenv ; i++) {
+if (safewrite(logfd, cmd-env[i], strlen(cmd-env[i])) 0)
+ioError = errno;
+if (safewrite(logfd, , 1) 0)
+ioError = errno;
+}
+for (i = 0 ; i cmd-nargs ; i++) {
+if (safewrite(logfd, cmd-args[i], strlen(cmd-args[i])) 0)
+ioError = errno;
+if (safewrite(logfd, i == cmd-nargs - 1 ? \n : , 1) 0)
+ioError = errno;
+}
As written this will only save the last occurred error in ioError. Is
this intended? Looks like a best effort approach, try to write as much
as possible ignoring errors.
Well, the function has no return value, so yes, that's the best we can
do - log as much as possible, and issue a VIR_WARN if we didn't log
everything. But I couldn't seem to justify returning an error and
stopping the log at the first error - how do you log that you had an
error logging, when logging is orthogonal to running the command in the
first place?
In virCommandProcessIO you say that cmd-{out|err}fd is a valid FD
(created in virExecWithHook called by virCommandRunAsync) when
cmd-{out|err}buf is not NULL. As far as I can tell from the code that
is true when the caller had requested to capture stdout and stderr.
But in case that caller didn't do this then you setup buffers to
capture stdout and stderr for logging. In that case
virCommandProcessIO will be called with cmd-{out|err}buf being
non-NULL but cmd-{out|err}fd being invalid as you explicitly set them
to -1 in the two if blocks before the call to virCommandProcessIO.
Yep, that needed reordering. If virCommandRun detects that nothing set
output, then outfd needs to be set prior to virCommandRunAsync so as to
force the creation of a pipe rather than assigning fds to /dev/null.
+
+#ifndef __linux__
What's Linux specific in this test? Shouldn't the command API and this
test be working on all systems that support fork/exec?
It should really be #if !HAVE_FORK (aka #ifdef WIN32).
I'm testing the impacts of squashing this in...
diff --git i/cfg.mk w/cfg.mk
index 8a8da18..29de9d9 100644
--- i/cfg.mk
+++ w/cfg.mk
@@ -369,9 +369,9 @@ msg_gen_function += umlReportError
msg_gen_function += vah_error
msg_gen_function += vah_warning
msg_gen_function += vboxError
+msg_gen_function += virCommandError
msg_gen_function += virConfError
msg_gen_function += virDomainReportError
-msg_gen_function += virSecurityReportError
msg_gen_function += virHashError
msg_gen_function += virLibConnError
msg_gen_function += virLibDomainError
@@ -380,6 +380,7 @@ msg_gen_function += virNodeDeviceReportError
msg_gen_function += virRaiseError
msg_gen_function += virReportErrorHelper
msg_gen_function += virReportSystemError
+msg_gen_function += virSecurityReportError
msg_gen_function += virSexprError
msg_gen_function += virStorageReportError
msg_gen_function += virXMLError
diff --git i/src/util/command.c w/src/util/command.c
index 948a957..aa43f76 100644
--- i/src/util/command.c
+++ w/src/util/command.c
@@ -91,7 +91,7 @@
Re: [libvirt] Looking for Hypervisor Vulerability Example
James Morris' presentation is referring to this published demonstration of exploiting Xen a few years ago http://www.securityfocus.com/archive/1/497376 http://invisiblethingslab.com/resources/misc08/xenfb-adventures-10.pdf The key difference sVirt makes is at chapter 3.4 in the paper. In Xen world, there was a single SELinux domain (xend_t) that covered XenD and all the QEMU processes. Since all VMs XenD ran as the same context, any exploited QEMU process in Xen, could access any other guest disks, as well as any host disks. In the KVM + sVirt world, every QEMU process is separated by a dedicated MCS category on its SELinux context. The disks assigned to a guest are labelled with the same MCS category. This means that an exploited QEMU can only access disks which were explicitly assigned to it, and cannot access the host disk devices. This prevents the step in that paper where they overwrite various key files in the host OS root filesystem Regards, Daniel Cool! Is there any well documented KVM exploit that can be reproduced without too much trouble, assuming SELinux (sVirt) is turned off? Then I can demonostrate the effect of sVirt by turning it on. Thank you very much. Shi -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 8/8] Remove bogus includes
On 12/02/2010 07:08 AM, Daniel P. Berrange wrote:
On Tue, Nov 23, 2010 at 04:50:01PM -0700, Eric Blake wrote:
From: Daniel P. Berrange berra...@redhat.com
---
v2: rearrange to later in the series; no other change. Passes
for me with macvtap compilation enabled, so I'm not sure if it
still suffers from the same problem as the v1 complaint:
https://www.redhat.com/archives/libvir-list/2010-November/msg00834.html
src/conf/domain_conf.c |1 -
src/util/hooks.c |1 -
2 files changed, 0 insertions(+), 2 deletions(-)
ACK
All right; I'm now pushing the amended series; below are the actual
differences from v2 that ended up being squashed in (mostly to 3/8, and
mostly in response to Matthias' comments).
The problem I hit was actually with removing
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 5dcc9e1..eb4ea8f 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -49,7 +49,6 @@
# include logging.h
# include macvtap.h
# include interface.h
-# include conf/domain_conf.h
Because the 'util' directory must never depend on the 'conf' directory.
That's a separate issue; it still needs to be resolved, but it is
unrelated to virCommand (so much as it happened to be a patch on the git
branch that I took from your tree when starting my improvements to
virCommand).
diff --git c/cfg.mk w/cfg.mk
index 8a8da18..29de9d9 100644
--- c/cfg.mk
+++ w/cfg.mk
@@ -369,9 +369,9 @@ msg_gen_function += umlReportError
msg_gen_function += vah_error
msg_gen_function += vah_warning
msg_gen_function += vboxError
+msg_gen_function += virCommandError
msg_gen_function += virConfError
msg_gen_function += virDomainReportError
-msg_gen_function += virSecurityReportError
msg_gen_function += virHashError
msg_gen_function += virLibConnError
msg_gen_function += virLibDomainError
@@ -380,6 +380,7 @@ msg_gen_function += virNodeDeviceReportError
msg_gen_function += virRaiseError
msg_gen_function += virReportErrorHelper
msg_gen_function += virReportSystemError
+msg_gen_function += virSecurityReportError
msg_gen_function += virSexprError
msg_gen_function += virStorageReportError
msg_gen_function += virXMLError
diff --git c/src/util/command.c w/src/util/command.c
index 948a957..aa43f76 100644
--- c/src/util/command.c
+++ w/src/util/command.c
@@ -91,7 +91,7 @@ virCommandNew(const char *binary)
/*
* Create a new command with a NULL terminated
- * set of args, taking binary from argv[0]
+ * set of args, taking binary from args[0]
*/
virCommandPtr
virCommandNewArgs(const char *const*args)
@@ -543,7 +543,7 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char
**outbuf)
if (!cmd || cmd-has_error)
return;
-if (cmd-outfd != -1) {
+if (cmd-outfdptr) {
cmd-has_error = -1;
VIR_DEBUG0(cannot specify output twice);
return;
@@ -563,7 +563,7 @@ virCommandSetErrorBuffer(virCommandPtr cmd, char
**errbuf)
if (!cmd || cmd-has_error)
return;
-if (cmd-errfd != -1) {
+if (cmd-errfdptr) {
cmd-has_error = -1;
VIR_DEBUG0(cannot specify stderr twice);
return;
@@ -583,11 +583,16 @@ virCommandSetInputFD(virCommandPtr cmd, int infd)
if (!cmd || cmd-has_error)
return;
-if (infd 0 || cmd-inbuf) {
+if (cmd-infd != -1 || cmd-inbuf) {
cmd-has_error = -1;
VIR_DEBUG0(cannot specify input twice);
return;
}
+if (infd 0) {
+cmd-has_error = -1;
+VIR_DEBUG0(cannot specify invalid input fd);
+return;
+}
cmd-infd = infd;
}
@@ -602,7 +607,7 @@ virCommandSetOutputFD(virCommandPtr cmd, int *outfd)
if (!cmd || cmd-has_error)
return;
-if (!outfd || cmd-outfd != -1) {
+if (cmd-outfdptr) {
cmd-has_error = -1;
VIR_DEBUG0(cannot specify output twice);
return;
@@ -621,7 +626,7 @@ virCommandSetErrorFD(virCommandPtr cmd, int *errfd)
if (!cmd || cmd-has_error)
return;
-if (!errfd || cmd-errfd != -1) {
+if (cmd-errfdptr) {
cmd-has_error = -1;
VIR_DEBUG0(cannot specify stderr twice);
return;
@@ -642,6 +647,11 @@ virCommandSetPreExecHook(virCommandPtr cmd,
virExecHook hook, void *opaque)
if (!cmd || cmd-has_error)
return;
+if (cmd-hook) {
+cmd-has_error = -1;
+VIR_DEBUG0(cannot specify hook twice);
+return;
+}
cmd-hook = hook;
cmd-opaque = opaque;
}
@@ -778,7 +788,7 @@ virCommandProcessIO(virCommandPtr cmd)
if (nfds == 0)
break;
-if (poll(fds,nfds, -1) 0) {
+if (poll(fds, nfds, -1) 0) {
if ((errno == EAGAIN) || (errno == EINTR))
continue;
virReportSystemError(errno, %s,
@@ -882,12 +892,25 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
if (pipe(infd) 0) {
virReportSystemError(errno, %s,
_(unable to open pipe));
+
Re: [libvirt] [PATCH] virsh: move two commands from domain group to storage pool group
On 12/01/2010 09:34 PM, Justin Clift wrote: On 02/12/2010, at 11:25 AM, Osier Yang wrote: * tools/virsh.c (find-storage-pool-sources-as and find-storage-pool-sources should't be in command group Domain Management, move them to group Storage Pool. --- tools/virsh.c |8 1 files changed, 4 insertions(+), 4 deletions(-) ACK. Now pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] make syntax-check - make: *** [sc_check_author_list] Error 1 on libvirt-0.8.5
[adding bug-gnulib, thanks to some issues with 'make syntax-check' in gnulib-provided files] On 12/02/2010 02:25 AM, Kenneth Nagin wrote: I am receiving syntax error when compiling libvirt-0.8.5. However, make without syntax-check completes successfully. check_author_list %aE maint.mk: committer(s) not listed in AUTHORS That means your version of git is too old to support the specific log formatting directive that we are using. What version of git are you using, and is it worth us fixing that syntax check to skip if git is too old? And ultimately, failure of 'make syntax-check' is non-fatal; it is not a prerequisite for building working binaries, so much as a way to try and enforce consistent style within the code base. I decided that it made more sense to simply work with 0.8.6 (rather than 0.8.5). But now I am getting another error, i.e Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6. It prints it a lot of times and then hangs. make without syntax-check works fine. Here are the error messages: You still haven't told me what 'git --version' outputs on your system. That may be the clue to solving all of this. na...@croton:~/LIBVIRT/libvirt-0.8.6$ make syntax-check GFDL_version ./build-aux/vc-list-files: Failed to determine type of version control used in /home/nagin/LIBVIRT/libvirt-0.8.6 Also caused if git is too old or missing. What does this output? sh -vx build-aux/vc-list-files m4 That will help me figure out whether vc-list-files needs to be patched, and or maint.mk taught to skip tests that require vc-list-files when run from a tarball rather than from a git checkout. Ultimately, 'make syntax-check' is intended for developers working from the latest development repository, and isn't really needed for end users working from a tarball. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 1/4] threadpool impl
On 12/02/2010 05:28 AM, Daniel P. Berrange wrote: +virThreadPoolPtr virThreadPoolNew(size_t minWorkers, + size_t maxWorkers, + virThreadPoolJobFunc func, + void *opaque) ATTRIBUTE_NONNULL(3) +ATTRIBUTE_RETURN_CHECK; ATTRIBUTE_RETURN_CHECK doesn't serve any useful purpose when placed on constructors, since the caller will always use the return value by assigning the pointer to some variable. The compiler can thus never detect whether you check for null or not, even with this annotation. Good point. However, in looking through gcc's documentation, maybe it's time we introduce a new attribute for constructors: #define ATTRIBUTE_MALLOC __attribute__((__malloc__)) The `malloc' attribute is used to tell the compiler that a function may be treated as if any non-`NULL' pointer it returns cannot alias any other pointer valid when the function returns. This will often improve optimization. Standard functions with this property include `malloc' and `calloc'. `realloc'-like functions have this property as long as the old pointer is never referred to (including comparing it to the new pointer) after the function returns a non-`NULL' value. I think that tools like clang might also be able to feed off of the malloc attribute to make decisions about whether NULL-checking needs to be performed on the result, and/or provide better leak detection analysis. However, that's a separate idea, and doesn't affect this series. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Create file in virFileWriteStr() if it doesn't exist
On 12/02/2010 06:23 AM, Jean-Baptiste Rouault wrote:
This patch adds a virFileWriteStrEx() function with a
third parameter to set the created file permissions.
I'm not a fan of Microsoft's naming conventions (Ex conveys no meaning,
and it doesn't scale well to future extensions[1]). Can we come up with
a better name, such as virFileWriteStrCreate?
[1] http://blogs.msdn.com/b/michkap/archive/2006/01/31/520694.aspx
Alternatively, I only counted 16 existing users of virFileWriteStr; and
this is an internal API. We could easily rewrite all clients to always
pass a third parameter, and change the signature of virFileWriteStr to
require a mode_t argument. Hmm; some of those clients are writing to
kernel files that should always exist (/proc/sys/net/ipv4/ip_forward,
for example), where it's tough to justify what we would pass as a mode_t
argument. So maybe pass mode==0 as a sentinel to require a pre-existing
file (that is, even though open(,O_CREAT,0) is a valid syscall, it
seldom makes sense, since the resulting created file cannot be re-read
once closed):
int virFileWriteStr(const char *path, const char *str, mode_t mode) {
int fd;
if (mode == 0)
fd = open(path, O_WRONLY|O_TRUNC);
else
fd = open(path, O_WRONLY|O_CREAT|O_TRUNC, mode);
if (fd == -1)
...
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: allow creation of file with virFileWriteStr
Making this change will allow the future patches to use
virFileWriteStr to create a file, rather than its current limitation
of only working on pre-existing files.
* src/util/util.h (virFileWriteStr): Alter signature.
* src/util/util.c (virFileWriteStr): Allow file creation.
* src/network/bridge_driver.c (networkEnableIpForwarding)
(networkDisableIPV6): Adjust clients.
* src/node_device/node_device_driver.c
(nodeDeviceVportCreateDelete): Likewise.
* src/util/cgroup.c (virCgroupSetValueStr): Likewise.
* src/util/pci.c (pciBindDeviceToStub, pciUnBindDeviceFromStub):
Likewise.
Based on a report from Jean-Baptiste Rouault.
---
Alternatively, I only counted 16 existing users of virFileWriteStr; and
this is an internal API. We could easily rewrite all clients to always
pass a third parameter, and change the signature of virFileWriteStr to
require a mode_t argument. Hmm; some of those clients are writing to
kernel files that should always exist (/proc/sys/net/ipv4/ip_forward,
for example), where it's tough to justify what we would pass as a mode_t
argument. So maybe pass mode==0 as a sentinel to require a pre-existing
file
How does this look? Admittedly, all existing uses were okay with a
mode parameter of 0; and I haven't yet seen your patch that would
use a non-zero mode, but this still makes more sense to me.
Oh, and VIR_FORCE_CLOSE preserves errno, so I was able to simplify
virFileWriteStr in the process.
src/network/bridge_driver.c |8
src/node_device/node_device_driver.c |2 +-
src/util/cgroup.c|2 +-
src/util/pci.c | 16
src/util/util.c | 13 -
src/util/util.h |3 ++-
6 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 54890f9..62639e4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1024,7 +1024,7 @@ networkReloadIptablesRules(struct network_driver *driver)
static int
networkEnableIpForwarding(void)
{
-return virFileWriteStr(/proc/sys/net/ipv4/ip_forward, 1\n);
+return virFileWriteStr(/proc/sys/net/ipv4/ip_forward, 1\n, 0);
}
#define SYSCTL_PATH /proc/sys
@@ -1045,7 +1045,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
goto cleanup;
}
-if (virFileWriteStr(field, 1) 0) {
+if (virFileWriteStr(field, 1, 0) 0) {
virReportSystemError(errno,
_(cannot enable %s), field);
goto cleanup;
@@ -1057,7 +1057,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
goto cleanup;
}
-if (virFileWriteStr(field, 0) 0) {
+if (virFileWriteStr(field, 0, 0) 0) {
virReportSystemError(errno,
_(cannot disable %s), field);
goto cleanup;
@@ -1069,7 +1069,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
goto cleanup;
}
-if (virFileWriteStr(field, 1) 0) {
+if (virFileWriteStr(field, 1, 0) 0) {
virReportSystemError(errno,
_(cannot enable %s), field);
goto cleanup;
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 448cfd3..a6c6042 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -454,7 +454,7 @@ nodeDeviceVportCreateDelete(const int parent_host,
goto cleanup;
}
-if (virFileWriteStr(operation_path, vport_name) == -1) {
+if (virFileWriteStr(operation_path, vport_name, 0) == -1) {
virReportSystemError(errno,
_(Write of '%s' to '%s' during
vport create/delete failed),
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 2758a8f..3ba6325 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -288,7 +288,7 @@ static int virCgroupSetValueStr(virCgroupPtr group,
return rc;
VIR_DEBUG(Set value '%s' to '%s', keypath, value);
-rc = virFileWriteStr(keypath, value);
+rc = virFileWriteStr(keypath, value, 0);
if (rc 0) {
DEBUG(Failed to write value '%s': %m, value);
rc = -errno;
diff --git a/src/util/pci.c b/src/util/pci.c
index d38cefa..095ad3f 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -849,7 +849,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
* bound by the stub.
*/
pciDriverFile(path, sizeof(path), driver, new_id);
-if (virFileWriteStr(path, dev-id) 0) {
+if (virFileWriteStr(path, dev-id, 0) 0) {
virReportSystemError(errno,
_(Failed to add PCI device ID '%s' to %s),
dev-id, driver);
@@ -862,7 +862,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
* your root filesystem.
*/
pciDeviceFile(path, sizeof(path), dev-name, driver/unbind);
-
[libvirt] [PATCH] openvz: convert popen to virCommand
popen must be matched with pclose (not fclose), or it will leak
resources. Furthermore, it is a lousy interface when it comes
to signal handling. We're much better off using our decent command
wrapper.
* src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID):
Replace popen with virCommand usage.
---
src/openvz/openvz_conf.c | 54 +++--
1 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 863af93..9d2689a 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -51,6 +51,7 @@
#include util.h
#include nodeinfo.h
#include files.h
+#include command.h
#define VIR_FROM_THIS VIR_FROM_OPENVZ
@@ -433,26 +434,26 @@ openvzFreeDriver(struct openvz_driver *driver)
int openvzLoadDomains(struct openvz_driver *driver) {
-FILE *fp;
int veid, ret;
char status[16];
char uuidstr[VIR_UUID_STRING_BUFLEN];
virDomainObjPtr dom = NULL;
char temp[50];
+char *outbuf = NULL;
+char *line;
+virCommandPtr cmd = NULL;
if (openvzAssignUUIDs() 0)
return -1;
-if ((fp = popen(VZLIST -a -ovpsid,status -H 2/dev/null, r)) == NULL)
{
-openvzError(VIR_ERR_INTERNAL_ERROR, %s, _(popen failed));
-return -1;
-}
-
-while (!feof(fp)) {
-if (fscanf(fp, %d %s\n, veid, status) != 2) {
-if (feof(fp))
-break;
+cmd = virCommandNewArgList(VZLIST, -a, -ovpsid,status, -H, NULL);
+virCommandSetOutputBuffer(cmd, outbuf);
+if (virCommandRun(cmd, NULL) 0)
+goto cleanup;
+line = *outbuf ? outbuf : NULL;
+while (line) {
+if (sscanf(line, %d %s\n, veid, status) != 2) {
openvzError(VIR_ERR_INTERNAL_ERROR, %s,
_(Failed to parse vzlist output));
goto cleanup;
@@ -526,9 +527,14 @@ int openvzLoadDomains(struct openvz_driver *driver) {
virDomainObjUnlock(dom);
dom = NULL;
+
+line = strchr(line, '\n');
+if (line)
+line++;
}
-VIR_FORCE_FCLOSE(fp);
+virCommandFree(cmd);
+VIR_FREE(outbuf);
return 0;
@@ -536,7 +542,8 @@ int openvzLoadDomains(struct openvz_driver *driver) {
virReportOOMError();
cleanup:
-VIR_FORCE_FCLOSE(fp);
+virCommandFree(cmd);
+VIR_FREE(outbuf);
if (dom)
virDomainObjUnref(dom);
return -1;
@@ -978,27 +985,22 @@ static int openvzAssignUUIDs(void)
*/
int openvzGetVEID(const char *name) {
-char *cmd;
+virCommandPtr cmd;
+char *outbuf;
int veid;
-FILE *fp;
bool ok;
-if (virAsprintf(cmd, %s %s -ovpsid -H, VZLIST, name) 0) {
-openvzError(VIR_ERR_INTERNAL_ERROR, %s,
-_(virAsprintf failed));
+cmd = virCommandNewArgList(VZLIST, name, -ovpsid, -H, NULL);
+virCommandSetOutputBuffer(cmd, outbuf);
+if (virCommandRun(cmd, NULL) 0) {
+virCommandFree(cmd);
return -1;
}
-fp = popen(cmd, r);
-VIR_FREE(cmd);
-
-if (fp == NULL) {
-openvzError(VIR_ERR_INTERNAL_ERROR, %s, _(popen failed));
-return -1;
-}
+virCommandFree(cmd);
+ok = sscanf(outbuf, %d\n, veid) == 1;
+VIR_FREE(outbuf);
-ok = fscanf(fp, %d\n, veid ) == 1;
-VIR_FORCE_FCLOSE(fp);
if (ok veid = 0)
return veid;
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 1/4] threadpool impl
On Thu, Dec 02, 2010 at 12:28:17PM +, Daniel P. Berrange wrote:
On Thu, Dec 02, 2010 at 03:26:57PM +0800, Hu Tao wrote:
+virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
+ size_t maxWorkers,
+ virThreadPoolJobFunc func,
+ void *opaque)
+{
+virThreadPoolPtr pool;
+size_t i;
+
+if (minWorkers maxWorkers)
+minWorkers = maxWorkers;
+
+if (VIR_ALLOC(pool) 0) {
+virReportOOMError();
+return NULL;
+}
+
+pool-jobList.head = NULL;
+pool-jobList.tail = pool-jobList.head;
+
+pool-jobFunc = func;
+pool-jobOpaque = opaque;
+
+if (virMutexInit(pool-mutex) 0)
+goto error;
+if (virCondInit(pool-cond) 0)
+goto error;
+if (virCondInit(pool-quit_cond) 0)
+goto error;
+
+if (VIR_ALLOC_N(pool-workers, minWorkers) 0)
+goto error;
+
+pool-maxWorkers = maxWorkers;
+for (i = 0; i minWorkers; i++) {
+if (virThreadCreate(pool-workers[i],
+true,
+virThreadPoolWorker,
+pool) 0) {
+virThreadPoolFree(pool);
+return NULL;
+}
+pool-nWorkers++;
+}
+
+return pool;
+
+error:
+VIR_FREE(pool-workers);
+ignore_value(virCondDestroy(pool-quit_cond));
+ignore_value(virCondDestroy(pool-cond));
+virMutexDestroy(pool-mutex);
+return NULL;
+}
This is leaking 'pool' on error. IMHO it is preferrable to
just call virThreadPoolDestroy, otherwise anytime we add
another field to virThreadPoolPtr struct, we have to consider
updating 2 cleanup paths.
Agree. Since it is in error clean path (thus thread pool is not yet
created) it doesn't matter to lock on a broken mutex or wait on a
broken cond.
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 1/4] threadpool impl
On Thu, Dec 02, 2010 at 12:28:17PM +, Daniel P. Berrange wrote:
...snip...
+
+if (virThreadCreate(pool-workers[pool-nWorkers - 1],
+true,
+virThreadPoolWorker,
+pool) 0) {
+pool-nWorkers--;
+goto error;
+}
Small typo, that check should != NULL, rather than 0.
Confused. Do you mean virThreadCreate() or VIR_ALLOC() below? But both
return int.
+}
+
+if (VIR_ALLOC(job) 0) {
+virReportOOMError();
+goto error;
+}
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
Daniel P. Berrange wrote: Concept wise, do you reckon something like this would work: + a new libvirt-announce mailing list, low trafic, purely for release announcements and similar Along with us announcing a 'release candidate build through it (instead of the present approach). If it looks good after a period of time (a week or something as you mentioned), then it gets re-released as the actual release. If something turns up significantly broken, then we respin as a release candidate 2 and repeat the process. I think this is really viable, because it implies we need another week prior to creating the pre-release where we do what we currently do with pre-release stabalization. With a monthly release cycle, taking 2 weeks todo a release is too much of an time sink. Perhaps a 24 hour release candidate period? I have a staging project in openSUSE Build Service that builds for all currently supported SuSE products, which is a wide range of capabilities wrt Policy Kit versions, hal vs udev, libnl, avahi versions, cap-ng, netcf, macvtap, virtualport, yajl, ... I can deploy a release candidate tarball to the libvirt package in this project quickly to test building across these various SuSE products. I suspect there is an opportunity for some automation here as well. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 RESEND 4/4] Using threadpool API to manage qemud worker
On Thu, Dec 02, 2010 at 12:42:19PM +, Daniel P. Berrange wrote:
...snip...
-
-/* Caller must hold server lock */
-static struct qemud_client *qemudPendingJob(struct qemud_server *server)
+static void qemudWorker(void *data, void *opaque ATTRIBUTE_UNUSED)
{
-int i;
-for (i = 0 ; i server-nclients ; i++) {
-virMutexLock(server-clients[i]-lock);
-if (server-clients[i]-dx) {
-/* Delibrately don't unlock client - caller wants the lock */
-return server-clients[i];
-}
-virMutexUnlock(server-clients[i]-lock);
-}
-return NULL;
-}
+struct qemud_client *client = data;
+struct qemud_client_message *msg;
-static void *qemudWorker(void *data)
-{
-struct qemud_worker *worker = data;
-struct qemud_server *server = worker-server;
+virMutexLock(client-lock);
It is neccessary to hold the lock on 'server' before obtaining a
lock on 'client'. The server lock can be released again immediately
if no longer needed.
I guess the reason is to avoid locking a being-freed client. But
client-refs has been already incremented by 1 right before the client
goes into thread pool, which insures the client against being freed.
...snip...
+if (!server-workerPool) {
+VIR_ERROR0(_(Failed to create thread pool));
+virMutexUnlock(server-lock);
+return NULL;
}
for (;!server-quitEventThread;) {
A small change in that we no longer kill off idle worker threads,
Thread pool can be improved to achieve this internally or provide an
interface for callers to force kill off idle worker threads.
but the improved simplicity of libvirtd code makes this a worthwhile
tradeoff. So looks good to me aside from the minor locking bug.
Regards,
Daniel
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 0/4] Support of auto-dump on watchdog event in libvirtd
This patch series adds a new watchdog action `dump' which lets libvirtd can do auto-dump when receiving a watchdog event from qemu guest. In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot. Changes: v5: - qemu_driver is passed into threadpool as opaque parameter rather than visit the global qemu_driver in worker function - same situation as above of server in libvirtd.c - also list auto_dump_path in src/qemu/libvirtd_qemu.aug and src/qemu/test_libvirtd_qemu.aug - check return value of qemuDomainObjEndJob for safety v4: - updated threadpool to follow libvirt naming style, use appropriate internals APIs, and hide the struct definitions from the header (by Daniel) - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump() - use thread pool in libvirtd (qemud worker) v3: - let default auto-dump dir be /var/lib/libvirt/qemu/dump Hu Tao (4): threadpool impl Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker cfg.mk |1 + daemon/libvirtd.c | 172 + daemon/libvirtd.h |2 + src/Makefile.am |1 + src/conf/domain_conf.c |1 + src/conf/domain_conf.h |1 + src/libvirt_private.syms|6 + src/qemu/libvirtd_qemu.aug |1 + src/qemu/qemu.conf |5 + src/qemu/qemu_conf.c| 16 +++- src/qemu/qemu_conf.h|5 + src/qemu/qemu_driver.c | 228 -- src/qemu/test_libvirtd_qemu.aug |2 + src/util/threadpool.c | 231 +++ src/util/threadpool.h | 48 15 files changed, 518 insertions(+), 202 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h -- 1.7.3 -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
On 03/12/2010, at 4:04 PM, Jim Fehlig wrote: I can deploy a release candidate tarball to the libvirt package in this project quickly to test building across these various SuSE products. I suspect there is an opportunity for some automation here as well. Whichever way we go, I think automation within a release or two, if not right away, would have to be near mandatory. Whatever the delay period is, there will be times someone discovers a bug/blocker/etc, and once fixed a new package needs to be created with everyone testing it again. If the testing itself takes ages, that would be non-optimal. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 4/4] Using threadpool API to manage qemud worker
---
daemon/libvirtd.c | 172 +
daemon/libvirtd.h |2 +
2 files changed, 31 insertions(+), 143 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 791b3dc..effa45f 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -67,6 +67,7 @@
#include stream.h
#include hooks.h
#include virtaudit.h
+#include threadpool.h
#ifdef HAVE_AVAHI
# include mdns.h
#endif
@@ -248,7 +249,6 @@ static void sig_handler(int sig, siginfo_t * siginfo,
static void qemudDispatchClientEvent(int watch, int fd, int events, void
*opaque);
static void qemudDispatchServerEvent(int watch, int fd, int events, void
*opaque);
-static int qemudStartWorker(struct qemud_server *server, struct qemud_worker
*worker);
void
qemudClientMessageQueuePush(struct qemud_client_message **queue,
@@ -1458,19 +1458,6 @@ static int qemudDispatchServer(struct qemud_server
*server, struct qemud_socket
server-clients[server-nclients++] = client;
-if (server-nclients server-nactiveworkers
-server-nactiveworkers server-nworkers) {
-for (i = 0 ; i server-nworkers ; i++) {
-if (!server-workers[i].hasThread) {
-if (qemudStartWorker(server, server-workers[i]) 0)
-return -1;
-server-nactiveworkers++;
-break;
-}
-}
-}
-
-
return 0;
error:
@@ -1534,100 +1521,28 @@ void qemudDispatchClientFailure(struct qemud_client
*client) {
VIR_FREE(client-addrstr);
}
-
-/* Caller must hold server lock */
-static struct qemud_client *qemudPendingJob(struct qemud_server *server)
-{
-int i;
-for (i = 0 ; i server-nclients ; i++) {
-virMutexLock(server-clients[i]-lock);
-if (server-clients[i]-dx) {
-/* Delibrately don't unlock client - caller wants the lock */
-return server-clients[i];
-}
-virMutexUnlock(server-clients[i]-lock);
-}
-return NULL;
-}
-
-static void *qemudWorker(void *data)
+static void qemudWorker(void *data, void *opaque)
{
-struct qemud_worker *worker = data;
-struct qemud_server *server = worker-server;
-
-while (1) {
-struct qemud_client *client = NULL;
-struct qemud_client_message *msg;
-
-virMutexLock(server-lock);
-while ((client = qemudPendingJob(server)) == NULL) {
-if (worker-quitRequest ||
-virCondWait(server-job, server-lock) 0) {
-virMutexUnlock(server-lock);
-return NULL;
-}
-}
-if (worker-quitRequest) {
-virMutexUnlock(client-lock);
-virMutexUnlock(server-lock);
-return NULL;
-}
-worker-processingCall = 1;
-virMutexUnlock(server-lock);
-
-/* We own a locked client now... */
-client-refs++;
-
-/* Remove our message from dispatch queue while we use it */
-msg = qemudClientMessageQueueServe(client-dx);
-
-/* This function drops the lock during dispatch,
- * and re-acquires it before returning */
-if (remoteDispatchClientRequest (server, client, msg) 0) {
-VIR_FREE(msg);
-qemudDispatchClientFailure(client);
-client-refs--;
-virMutexUnlock(client-lock);
-continue;
-}
-
-client-refs--;
-virMutexUnlock(client-lock);
-
-virMutexLock(server-lock);
-worker-processingCall = 0;
-virMutexUnlock(server-lock);
-}
-}
-
-static int qemudStartWorker(struct qemud_server *server,
-struct qemud_worker *worker) {
-pthread_attr_t attr;
-pthread_attr_init(attr);
-/* We want to join workers, so don't detach them */
-/*pthread_attr_setdetachstate(attr, 1);*/
+struct qemud_server *server = opaque;
+struct qemud_client *client = data;
+struct qemud_client_message *msg;
-if (worker-hasThread)
-return -1;
+virMutexLock(client-lock);
-worker-server = server;
-worker-hasThread = 1;
-worker-quitRequest = 0;
-worker-processingCall = 0;
+/* Remove our message from dispatch queue while we use it */
+msg = qemudClientMessageQueueServe(client-dx);
-if (pthread_create(worker-thread,
- attr,
- qemudWorker,
- worker) != 0) {
-worker-hasThread = 0;
-worker-server = NULL;
-return -1;
+/* This function drops the lock during dispatch,
+ * and re-acquires it before returning */
+if (remoteDispatchClientRequest (server, client, msg) 0) {
+VIR_FREE(msg);
+qemudDispatchClientFailure(client);
}
-return 0;
+client-refs--;
+virMutexUnlock(client-lock);
}
-
/*
* Read data into buffer using wire decoding (plain or TLS)
*
@@ -1857,8 +1772,11 @@ readmore:
}
/* Move
[libvirt] [PATCH v5 3/4] Add a watchdog action `dump'
`dump' watchdog action lets libvirtd to dump the guest when receives a
watchdog event (which probably means a guest crash)
Currently only qemu is supported.
---
src/conf/domain_conf.c |1 +
src/conf/domain_conf.h |1 +
src/qemu/libvirtd_qemu.aug |1 +
src/qemu/qemu.conf |5 ++
src/qemu/qemu_conf.c| 16 ++-
src/qemu/qemu_conf.h|5 ++
src/qemu/qemu_driver.c | 96 +++
src/qemu/test_libvirtd_qemu.aug |2 +
8 files changed, 126 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3f14cee..a6cb444 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -245,6 +245,7 @@ VIR_ENUM_IMPL(virDomainWatchdogAction,
VIR_DOMAIN_WATCHDOG_ACTION_LAST,
shutdown,
poweroff,
pause,
+ dump,
none)
VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 899b19f..7f50b79 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -462,6 +462,7 @@ enum virDomainWatchdogAction {
VIR_DOMAIN_WATCHDOG_ACTION_SHUTDOWN,
VIR_DOMAIN_WATCHDOG_ACTION_POWEROFF,
VIR_DOMAIN_WATCHDOG_ACTION_PAUSE,
+VIR_DOMAIN_WATCHDOG_ACTION_DUMP,
VIR_DOMAIN_WATCHDOG_ACTION_NONE,
VIR_DOMAIN_WATCHDOG_ACTION_LAST
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 78852f3..2f37015 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -37,6 +37,7 @@ module Libvirtd_qemu =
| str_array_entry cgroup_device_acl
| str_entry save_image_format
| str_entry dump_image_format
+ | str_entry auto_dump_path
| str_entry hugetlbfs_mount
| bool_entry relaxed_acs_check
| bool_entry vnc_allow_host_audio
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f4f965e..ba41f80 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -191,6 +191,11 @@
# save_image_format = raw
# dump_image_format = raw
+# When a domain is configured to be auto-dumped when libvirtd receives a
+# watchdog event from qemu guest, libvirtd will save dump files in directory
+# specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump
+#
+# auto_dump_path = /var/lib/libvirt/qemu/dump
# If provided by the host and a hugetlbfs mount point is configured,
# a guest may request huge page backing. When this mount point is
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7cd0603..187e206 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -386,6 +386,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
}
}
+p = virConfGetValue (conf, auto_dump_path);
+CHECK_TYPE (auto_dump_path, VIR_CONF_STRING);
+if (p p-str) {
+VIR_FREE(driver-autoDumpPath);
+if (!(driver-autoDumpPath = strdup(p-str))) {
+virReportOOMError();
+virConfFree(conf);
+return -1;
+}
+}
+
p = virConfGetValue (conf, hugetlbfs_mount);
CHECK_TYPE (hugetlbfs_mount, VIR_CONF_STRING);
if (p p-str) {
@@ -5374,7 +5385,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
ADD_ARG(optstr);
-const char *action =
virDomainWatchdogActionTypeToString(watchdog-action);
+int act = watchdog-action;
+if (act == VIR_DOMAIN_WATCHDOG_ACTION_DUMP)
+act = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE;
+const char *action = virDomainWatchdogActionTypeToString(act);
if (!action) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(invalid watchdog action));
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index aba64d6..9bcae88 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -41,6 +41,7 @@
# include driver.h
# include bitmap.h
# include macvtap.h
+# include threadpool.h
# define qemudDebug(fmt, ...) do {} while(0)
@@ -107,6 +108,8 @@ enum qemud_cmd_flags {
struct qemud_driver {
virMutex lock;
+virThreadPoolPtr workerPool;
+
int privileged;
uid_t user;
@@ -174,6 +177,8 @@ struct qemud_driver {
char *saveImageFormat;
char *dumpImageFormat;
+char *autoDumpPath;
+
pciDeviceList *activePciHostdevs;
virBitmapPtr reservedVNCPorts;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e534195..713179b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -85,6 +85,7 @@
#include files.h
#include fdstream.h
#include configmake.h
+#include threadpool.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -137,6 +138,14 @@ struct _qemuDomainObjPrivate {
int persistentAddrs;
};
+struct watchdogEvent
+{
+virDomainObjPtr vm;
+int action;
+};
+
+static void
[libvirt] [PATCH v5 1/4] threadpool impl
* src/util/threadpool.c, src/util/threadpool.h: Thread pool
implementation
* src/Makefile.am: Build thread pool
* src/libvirt_private.syms: Export public functions
---
cfg.mk |1 +
src/Makefile.am |1 +
src/libvirt_private.syms |6 +
src/util/threadpool.c| 231 ++
src/util/threadpool.h| 48 ++
5 files changed, 287 insertions(+), 0 deletions(-)
create mode 100644 src/util/threadpool.c
create mode 100644 src/util/threadpool.h
diff --git a/cfg.mk b/cfg.mk
index 6e474c4..0af26d2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -127,6 +127,7 @@ useless_free_options = \
--name=virStoragePoolObjFree \
--name=virStoragePoolSourceFree \
--name=virStorageVolDefFree \
+ --name=virThreadPoolFree \
--name=xmlFree \
--name=xmlXPathFreeContext \
--name=xmlXPathFreeObject
diff --git a/src/Makefile.am b/src/Makefile.am
index a9a1986..d71c644 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -73,6 +73,7 @@ UTIL_SOURCES =
\
util/threads.c util/threads.h \
util/threads-pthread.h \
util/threads-win32.h\
+ util/threadpool.c util/threadpool.h \
util/uuid.c util/uuid.h \
util/util.c util/util.h \
util/xml.c util/xml.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a21928a..e7b500c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -862,3 +862,9 @@ virXPathStringLimit;
virXPathULong;
virXPathULongHex;
virXPathULongLong;
+
+
+# threadpool.h
+virThreadPoolNew;
+virThreadPoolFree;
+virThreadPoolSendJob;
diff --git a/src/util/threadpool.c b/src/util/threadpool.c
new file mode 100644
index 000..8217591
--- /dev/null
+++ b/src/util/threadpool.c
@@ -0,0 +1,231 @@
+/*
+ * threadpool.c: a generic thread pool implementation
+ *
+ * Copyright (C) 2010 Hu Tao
+ * Copyright (C) 2010 Daniel P. Berrange
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors:
+ * Hu Tao hu...@cn.fujitsu.com
+ * Daniel P. Berrange berra...@redhat.com
+ */
+
+#include config.h
+
+#include threadpool.h
+#include memory.h
+#include threads.h
+#include virterror_internal.h
+#include ignore-value.h
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+typedef struct _virThreadPoolJob virThreadPoolJob;
+typedef virThreadPoolJob *virThreadPoolJobPtr;
+
+struct _virThreadPoolJob {
+virThreadPoolJobPtr next;
+
+void *data;
+};
+
+typedef struct _virThreadPoolJobList virThreadPoolJobList;
+typedef virThreadPoolJobList *virThreadPoolJobListPtr;
+
+struct _virThreadPoolJobList {
+virThreadPoolJobPtr head;
+virThreadPoolJobPtr *tail;
+};
+
+
+struct _virThreadPool {
+bool quit;
+
+virThreadPoolJobFunc jobFunc;
+void *jobOpaque;
+virThreadPoolJobList jobList;
+
+virMutex mutex;
+virCond cond;
+virCond quit_cond;
+
+size_t maxWorkers;
+size_t freeWorkers;
+size_t nWorkers;
+virThreadPtr workers;
+};
+
+static void virThreadPoolWorker(void *opaque)
+{
+virThreadPoolPtr pool = opaque;
+
+virMutexLock(pool-mutex);
+
+while (1) {
+while (!pool-quit
+ !pool-jobList.head) {
+pool-freeWorkers++;
+if (virCondWait(pool-cond, pool-mutex) 0) {
+pool-freeWorkers--;
+goto out;
+}
+pool-freeWorkers--;
+}
+
+if (pool-quit)
+break;
+
+virThreadPoolJobPtr job = pool-jobList.head;
+pool-jobList.head = pool-jobList.head-next;
+job-next = NULL;
+if (pool-jobList.tail == job-next)
+pool-jobList.tail = pool-jobList.head;
+
+virMutexUnlock(pool-mutex);
+(pool-jobFunc)(job-data, pool-jobOpaque);
+VIR_FREE(job);
+virMutexLock(pool-mutex);
+}
+
+out:
+pool-nWorkers--;
+if (pool-nWorkers == 0)
+
[libvirt] [PATCH v5 2/4] Add a new function doCoreDump
This patch prepares for the next patch.
---
src/qemu/qemu_driver.c | 132 +++-
1 files changed, 74 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1a7c1ad..e534195 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6063,6 +6063,78 @@ cleanup:
return ret;
}
+static int doCoreDump(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ const char *path,
+ enum qemud_save_formats compress)
+{
+int fd = -1;
+int ret = -1;
+qemuDomainObjPrivatePtr priv;
+
+priv = vm-privateData;
+
+/* Create an empty file with appropriate ownership. */
+if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(failed to create '%s'), path);
+goto cleanup;
+}
+
+if (VIR_CLOSE(fd) 0) {
+virReportSystemError(errno,
+ _(unable to save file %s),
+ path);
+goto cleanup;
+}
+
+if (driver-securityDriver
+driver-securityDriver-domainSetSavedStateLabel
+
driver-securityDriver-domainSetSavedStateLabel(driver-securityDriver,
+ vm, path) == -1)
+goto cleanup;
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+if (compress == QEMUD_SAVE_FORMAT_RAW) {
+const char *args[] = {
+cat,
+NULL,
+};
+ret = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
+} else {
+const char *prog = qemudSaveCompressionTypeToString(compress);
+const char *args[] = {
+prog,
+-c,
+NULL,
+};
+ret = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
+}
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+if (ret 0)
+goto cleanup;
+
+ret = qemuDomainWaitForMigrationComplete(driver, vm);
+
+if (ret 0)
+goto cleanup;
+
+if (driver-securityDriver
+driver-securityDriver-domainRestoreSavedStateLabel
+
driver-securityDriver-domainRestoreSavedStateLabel(driver-securityDriver,
+ vm, path) == -1)
+goto cleanup;
+
+cleanup:
+if (ret != 0)
+unlink(path);
+return ret;
+}
+
static enum qemud_save_formats
getCompressionType(struct qemud_driver *driver)
{
@@ -6097,13 +6169,10 @@ static int qemudDomainCoreDump(virDomainPtr dom,
struct qemud_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
int resume = 0, paused = 0;
-int ret = -1, fd = -1;
+int ret = -1;
virDomainEventPtr event = NULL;
-enum qemud_save_formats compress;
qemuDomainObjPrivatePtr priv;
-compress = getCompressionType(driver);
-
qemuDriverLock(driver);
vm = virDomainFindByUUID(driver-domains, dom-uuid);
@@ -6125,26 +6194,6 @@ static int qemudDomainCoreDump(virDomainPtr dom,
goto endjob;
}
-/* Create an empty file with appropriate ownership. */
-if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) 0) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-_(failed to create '%s'), path);
-goto endjob;
-}
-
-if (VIR_CLOSE(fd) 0) {
-virReportSystemError(errno,
- _(unable to save file %s),
- path);
-goto endjob;
-}
-
-if (driver-securityDriver
-driver-securityDriver-domainSetSavedStateLabel
-
driver-securityDriver-domainSetSavedStateLabel(driver-securityDriver,
- vm, path) == -1)
-goto endjob;
-
/* Migrate will always stop the VM, so the resume condition is
independent of whether the stop command is issued. */
resume = (vm-state == VIR_DOMAIN_RUNNING);
@@ -6168,43 +6217,12 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
}
-qemuDomainObjEnterMonitorWithDriver(driver, vm);
-if (compress == QEMUD_SAVE_FORMAT_RAW) {
-const char *args[] = {
-cat,
-NULL,
-};
-ret = qemuMonitorMigrateToFile(priv-mon,
- QEMU_MONITOR_MIGRATE_BACKGROUND,
- args, path, 0);
-} else {
-const char *prog = qemudSaveCompressionTypeToString(compress);
-const char *args[] = {
-prog,
--c,
-NULL,
-};
-ret = qemuMonitorMigrateToFile(priv-mon,
-
[libvirt] directory storage pools reported available space
We recently had an issue with not being able to allocate the full capacity of a directory based storage pool. The reported value via pool-info was larger than what was available to the image creator. Looking at the storage code, in virStorageBackendFileSystemRefresh() we're using statvfs, and reporting back pool-def-available = ((unsigned long long)sb.f_bfree * (unsigned long long)sb.f_bsize); Which is the amount of blocks available, including any root reservation if present on the filesystem. This does't line up with df output , which at least on RHEL5 and 6 systems reports the available space from f_bavail, which excludes and reserved space. Is it reasonable to have the available value line up with output from df and not report reserved space? -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] directory storage pools reported available space
于 2010年12月03日 14:00, Ryan Harper 写道: We recently had an issue with not being able to allocate the full capacity of a directory based storage pool. The reported value via pool-info was larger than what was available to the image creator. Looking at the storage code, in virStorageBackendFileSystemRefresh() we're using statvfs, and reporting back pool-def-available = ((unsigned long long)sb.f_bfree * (unsigned long long)sb.f_bsize); Which is the amount of blocks available, including any root reservation if present on the filesystem. This does't line up with df output , which at least on RHEL5 and 6 systems reports the available space from f_bavail, which excludes and reserved space. Is it reasonable to have the available value line up with output from df and not report reserved space? It's misleading not to exclude the reserved space, probly it will be nicer to report both the actually avaiable spaces and the reserved ones. - Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt 0.8.6
On 12/02/2010 03:03 PM, Diego Elio Pettenò wrote: Il giorno gio, 02/12/2010 alle 10.58 +, Daniel P. Berrange ha scritto: I think this is really viable, because it implies we need another week prior to creating the pre-release where we do what we currently do with pre-release stabalization. Note that I said “one week or less”. Less? I'm not sure if everybody can react that fast. I mean there is more to life than just: uh, new libvirt is out. I have to drop everything and test it; j/k :) - A official list of supported platforms / OS combinations - Run a test build on each combination before release - Actually follow the 'bug fixes' only rule leading upto release no matter how simple the new feature might appear. And let me guess that the supported platforms are going to be mostly RedHatco.? Why not warning the rest of the packagers, so that each run their tests? This seems to be perfectly fine. You, ehm guys, can't possibly to test every flavor of GNU/Linux. So it seems to be pretty much okay to me. It's up to community around each GNU/Linux distribution to do their own testing. Just my $0.02USD Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] man pages: update the description for the virsh help command
On 03/12/2010, at 4:47 AM, Eric Blake wrote: On the other hand, I'm thinking we implemented the help command slightly wrong by specifying that it takes two optional strings. Really, it only takes one optional string, which is a command-or-group. For instance, with the current code, 'virsh help --group help' lists a command help, rather than a group help, and 'virsh help --command virsh' lists the group help, rather than a command help. Meanwhile, 'virsh help help virsh' is accepted by the parser, but silently ignores the virsh group argument. Yeah, the current approach is a bit wrong. It can take either the string --command or --group, both of which do exactly the same thing rather than making a distinction. So I'm thinking we need yet another patch to virsh.c that reduces opts_help to just one VSH_OT_DATA flag name (whether we keep it named --command, or rename it to --command-or-group, is another question, which is also impacted by whether we decide to implement unambiguous prefix parsing like getopt_long). Good point. With the naming of the new option, I'm not sure how much stock we should put in maintaining backwards compatibility in this instance. With a new patch we could probably drop the --group keyword, plus make the --command keyword a null operation (ie ignore it). Then, if a command or group has been given on the line, display that as is presently being done. In the meantime, how about we list this line as: =item Bhelp optional Icommand-or-group ACK with that one-line change; the rest of the patch is uncontroversial, and the virsh.c cleanup can be a separate patch. That's better wording, thanks Eric. I'll make that tweak and push it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] man pages: update the description for the virsh help command
于 2010年12月03日 01:47, Eric Blake 写道: On 12/02/2010 06:04 AM, Justin Clift wrote: Now includes information on keyword usage, and provides examples. --- tools/virsh.pod | 37 ++--- 1 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index c97786a..66654a7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -115,10 +115,41 @@ The following commands are generic i.e. not specific to a domain. =over 4 -=item Bhelp optional Icommand +=item Bhelp optional I--command Icommand | Igroup-keyword Hmm, this doesn't quite match 'virsh help help', which if used literally would translate to: =item Bhelp optional Icommand Igroup On the other hand, I'm thinking we implemented the help command slightly wrong by specifying that it takes two optional strings. Really, it only takes one optional string, which is a command-or-group. For instance, with the current code, 'virsh help --group help' lists a command help, rather than a group help, and 'virsh help --command virsh' lists the group help, rather than a command help. Meanwhile, 'virsh help help virsh' is accepted by the parser, but silently ignores the virsh group argument. So I'm thinking we need yet another patch to virsh.c that reduces opts_help to just one VSH_OT_DATA flag name (whether we keep it named --command, or rename it to --command-or-group, is another question, which is also impacted by whether we decide to implement unambiguous prefix parsing like getopt_long). In the meantime, how about we list this line as: =item Bhelp optional Icommand-or-group ACK with that one-line change; the rest of the patch is uncontroversial, and the virsh.c cleanup can be a separate patch. I thought command-or-group is too long before, so didn't use it, now I change the mind, as think user nearly won't use it, they could directly get the help by # virsh help $word, where it's long or short doesn't matter much then. so I will make patch to use command-or-group Thanks - Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp
Remove the optional option group, as cmdHelp should accepts
only one option, and rename option command as command-or-group,
(virsh help supports both command and command group now, and user
nealy uses the options, so it doesn't matter much for it being longer,
:-)
* tools/virsh.c
---
tools/virsh.c |8 ++--
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 31de80f..c2d717f 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -569,8 +569,7 @@ static const vshCmdInfo info_help[] = {
};
static const vshCmdOptDef opts_help[] = {
-{command, VSH_OT_DATA, 0, N_(Prints global help or command specific
help.)},
-{group, VSH_OT_DATA, 0, N_(Prints global help or help for a group of
related commands.)},
+{command-or-group, VSH_OT_DATA, 0, N_(Prints global help, command
specific help, or help for a group of related commands)},
{NULL, 0, 0, NULL}
};
@@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
const vshCmdGrp *g;
const char *name;
-name = vshCommandOptString(cmd, command, NULL);
-
-if (!name)
-name = vshCommandOptString(cmd, group, NULL);
+name = vshCommandOptString(cmd, command-or-group, NULL);
if (!name) {
const vshCmdGrp *grp;
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] man pages: update the description for the virsh help command
于 2010年12月03日 14:50, Justin Clift 写道: On 03/12/2010, at 4:47 AM, Eric Blake wrote: On the other hand, I'm thinking we implemented the help command slightly wrong by specifying that it takes two optional strings. Really, it only takes one optional string, which is a command-or-group. For instance, with the current code, 'virsh help --group help' lists a command help, rather than a group help, and 'virsh help --command virsh' lists the group help, rather than a command help. Meanwhile, 'virsh help help virsh' is accepted by the parser, but silently ignores the virsh group argument. Yeah, the current approach is a bit wrong. It can take either the string --command or --group, both of which do exactly the same thing rather than making a distinction. So I'm thinking we need yet another patch to virsh.c that reduces opts_help to just one VSH_OT_DATA flag name (whether we keep it named --command, or rename it to --command-or-group, is another question, which is also impacted by whether we decide to implement unambiguous prefix parsing like getopt_long). Good point. With the naming of the new option, I'm not sure how much stock we should put in maintaining backwards compatibility in this instance. With a new patch we could probably drop the --group keyword, plus make the --command keyword a null operation (ie ignore it). Then, if a command or group has been given on the line, display that as is presently being done. In the meantime, how about we list this line as: =item Bhelp optional Icommand-or-group ACK with that one-line change; the rest of the patch is uncontroversial, and the virsh.c cleanup can be a separate patch. That's better wording, thanks Eric. I'll make that tweak and push it. great, I just sent patches with consistent changes. - Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
