Re: [libvirt] PATCH: Disable QEMU drive caching
On Wed, Oct 08, 2008 at 10:51:16AM -0500, Anthony Liguori wrote: Daniel P. Berrange wrote: - It is unsafe on host OS crash - all unflushed guest I/O will be lost, and there's no ordering guarentees, so metadata updates could be flushe to disk, while the journal updates were not. Say goodbye to your filesystem. This has nothing to do with cache=off. The IDE device defaults to write-back caching. As such, IDE makes no guarantee that when a data write completes, it's actually completed on disk. This only comes into play when write-back is disabled. I'm perfectly happy to accept a patch that adds explicit sync's when write-back is disabled. For SCSI, an unordered queue is advertised. Again, everything depends on whether or not write-back caching is enabled or not. Again, perfectly happy to take patches here. More importantly, the most common journaled filesystem, ext3, does not enable write barriers by default (even for journal updates). This is how it ship in Red Hat distros. So there is no greater risk of corrupting a journal in QEMU than there is on bare metal. Interesting discussion, I'm wondering about the non-local storage effect though, if the Node is caching writes, how can we ensure a coherent view on remote storage for example when migrating a domain ? Maybe migration is easy to fix because qemu is aware and can issue a sync, but as we start adding cloning APIs to libvirt, we could face the issue if issuing an LVM snapshot operation on the guest storage while the Node still cache some of the data. The more layers of caching the harder it is to have a predictable behaviour, no ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Daniel Veillard wrote: On Wed, Oct 08, 2008 at 10:51:16AM -0500, Anthony Liguori wrote: Daniel P. Berrange wrote: - It is unsafe on host OS crash - all unflushed guest I/O will be lost, and there's no ordering guarentees, so metadata updates could be flushe to disk, while the journal updates were not. Say goodbye to your filesystem. This has nothing to do with cache=off. The IDE device defaults to write-back caching. As such, IDE makes no guarantee that when a data write completes, it's actually completed on disk. This only comes into play when write-back is disabled. I'm perfectly happy to accept a patch that adds explicit sync's when write-back is disabled. For SCSI, an unordered queue is advertised. Again, everything depends on whether or not write-back caching is enabled or not. Again, perfectly happy to take patches here. More importantly, the most common journaled filesystem, ext3, does not enable write barriers by default (even for journal updates). This is how it ship in Red Hat distros. So there is no greater risk of corrupting a journal in QEMU than there is on bare metal. Interesting discussion, I'm wondering about the non-local storage effect though, if the Node is caching writes, how can we ensure a coherent view on remote storage for example when migrating a domain ? Maybe migration is easy to fix because qemu is aware and can issue a sync, but as we start adding cloning APIs to libvirt, we could face the issue if issuing an LVM snapshot operation on the guest storage while the Node still cache some of the data. The more layers of caching the harder it is to have a predictable behaviour, no ? Any live migration infrastructure must guarantee the write ordering between guest writes generated on the old node and guest writes generated on the new node. This usually happens as the live migration crosses the point of no return where the guest is allow to execute code on the new node. The old node must flush it's writes and/or the new node must delay any new writes until it is safe to do so. In the case of LVM snapshots, only one node is able to safely access the snapshot at a time, so an organized transfer of the active snapshot is necessary during the live migration. For the case of CLVM, I would think the cluster-aware bits would coordinate the transfer. Even in this case though, the data must be flushed out of the page cache on the old node and onto the storage itself. Steve -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Mark McLoughlin wrote: i.e. with write-back caching enabled, the IDE protocol makes no guarantees about when data is committed to disk. So, from a protocol correctness POV, qemu is behaving correctly with cache=on and write-back caching enabled on the disk. Yes, the host page cache is basically a big on-disk cache. For SCSI, an unordered queue is advertised. Again, everything depends on whether or not write-back caching is enabled or not. Again, perfectly happy to take patches here. Queue ordering and write-back caching sound like very different things. Are they two distinct SCSI options, or ...? Yes. Surely an ordered queue doesn't do much help prevent fs corruption if the host crashes, right? You would still need write-back caching disabled? You need both. In theory, a guest would use queue ordering to guarantee that certain writes made it to disk before other writes. Enabling write-through guarantees that the data is actually on disk. Since we advertise an unordered queue, we're okay from a safety point-of-view but for performance reasons, we'll want to do ordered queuing. More importantly, the most common journaled filesystem, ext3, does not enable write barriers by default (even for journal updates). This is how it ship in Red Hat distros. i.e. implementing barriers for virtio won't help most ext3 deployments? Yes, ext3 doesn't use barriers by default. See http://kerneltrap.org/mailarchive/linux-kernel/2008/5/19/1865314 And again, if barriers are just about ordering, don't you need to disable caching anyway? Well, virtio doesn't have a notion of write-caching. My thinking is that we ought to implement barriers via fdatasync because posix-aio already has an op for it. This would effectively use barriers as a point to force something on disk. I think this would take care of most of the data corruption issues since the cases where the guest cares about data corruption would be handled (barriers should be used for journal writes and any O_DIRECT write for instance, although, yeah, not the case today with ext3). So there is no greater risk of corrupting a journal in QEMU than there is on bare metal. This is the bit I really don't buy - we're equating qemu caching to IDE write-back caching and saying the risk of corruption is the same in both cases. Yes. But doesn't qemu cache data for far, far longer than a typical IDE disk with write-back caching would do? Doesn't that mean you're far, far more likely to see fs corruption with qemu caching? It caches more data, I don't know how much longer it cases than a typical IDE disk. The guest can crash and that won't cause data loss. The only thing that will really cause data loss is the host crashing so it's slightly better than write-back caching from that regard. Or put it another way, if we fix it by implementing the disabling of write-back caching ... users running a virtual machine will need to run hdparam -W 0 /dev/sda where they would never have run it on baremetal? I don't see it as something needing to be fixed because I don't see that the exposure is significantly greater for a VM than for a real machine. And let's take a step back too. If people are really concerned about this point, let's introduce a sync=on option that opens the image with O_SYNC. This will effectively make the cache write-through without the baggage associated with O_DIRECT. While I object to libvirt always setting cache=off, I think sync=on for IDE and SCSI may be reasonable (you don't want it for virtio-blk once we implement proper barriers with fdatasync I think). Regards, Anthony Liguori Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Daniel Veillard wrote: On Wed, Oct 08, 2008 at 10:51:16AM -0500, Anthony Liguori wrote: Daniel P. Berrange wrote: - It is unsafe on host OS crash - all unflushed guest I/O will be lost, and there's no ordering guarentees, so metadata updates could be flushe to disk, while the journal updates were not. Say goodbye to your filesystem. This has nothing to do with cache=off. The IDE device defaults to write-back caching. As such, IDE makes no guarantee that when a data write completes, it's actually completed on disk. This only comes into play when write-back is disabled. I'm perfectly happy to accept a patch that adds explicit sync's when write-back is disabled. For SCSI, an unordered queue is advertised. Again, everything depends on whether or not write-back caching is enabled or not. Again, perfectly happy to take patches here. More importantly, the most common journaled filesystem, ext3, does not enable write barriers by default (even for journal updates). This is how it ship in Red Hat distros. So there is no greater risk of corrupting a journal in QEMU than there is on bare metal. Interesting discussion, I'm wondering about the non-local storage effect though, if the Node is caching writes, how can we ensure a coherent view on remote storage for example when migrating a domain ? In the case of remote storage, cache coherency is part of the network storage protocol/architecture. In NFS for instance, the most common coherency model is close-to-open. Other network storage solutions provide stronger coherency models. Maybe migration is easy to fix because qemu is aware and can issue a sync, but as we start adding cloning APIs to libvirt, we could face the issue if issuing an LVM snapshot operation on the guest storage while the Node still cache some of the data. The more layers of caching the harder it is to have a predictable behaviour, no ? With respect to migration, QEMU does a flush(), but not an fdatasync. Even if we did an fdatasync, I'm not sure that's good enough with NFS because I don't know if fdatasync on the source *after* the target has opened a file and read data will guarantee consistency. Regards, Anthony Liguori Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Anthony Liguori wrote:
Mark McLoughlin wrote:
And let's take a step back too. If people are really concerned about
this point, let's introduce a sync=on option that opens the image with
O_SYNC. This will effectively make the cache write-through without
the baggage associated with O_DIRECT.
I'm starting to slowly convince myself we should always open files with
O_SYNC. Barriers should just force ordering within the thread pool.
posix-aio has no interface for this but we could create one with our own
thread pool implementation.
Ryan: could you give the following patch a perf-run so we can see how
this would effect us?
Thanks,
Anthony Liguori
While I object to libvirt always setting cache=off, I think sync=on
for IDE and SCSI may be reasonable (you don't want it for virtio-blk
once we implement proper barriers with fdatasync I think).
Regards,
Anthony Liguori
Cheers,
Mark.
diff --git a/block-raw-posix.c b/block-raw-posix.c
index c0404fa..0d8ffdb 100644
--- a/block-raw-posix.c
+++ b/block-raw-posix.c
@@ -121,7 +121,7 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags)
s-lseek_err_cnt = 0;
-open_flags = O_BINARY;
+open_flags = O_BINARY | O_SYNC;
if ((flags BDRV_O_ACCESS) == O_RDWR) {
open_flags |= O_RDWR;
} else {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Support SDL configuration for QEMU driver
Daniel P. Berrange [EMAIL PROTECTED] wrote:
On Thu, Oct 02, 2008 at 07:37:38PM +0200, Jim Meyering wrote:
Daniel P. Berrange [EMAIL PROTECTED] wrote:
...
Now previously since we just use 'execv' the QEMU process would just
inherit all libvirtd's environment variables. When we now use execve()
no variables are inherited - we have to explicitly set all the ones
we need. I'm not sure what we should consider the mimimum required?
I'm merely setting 'LC_ALL=C' to ensure it runs in C locale. Do we
need to set $PATH for QEMU - maybe ? Anything else which is good
practice to set ?
If QEMU uses PATH, then propagating that is necessary.
I guess it's debatable whether to use PATH=$PATH or
to use some hard-coded default on the RHS. But using PATH=$PATH
seems friendlier, in case whatever QEMU uses is in some non-default
location.
If it uses mkstemp or the like, then including TMPDIR would be good.
Depending on QEMU, maybe things like HOME, USER, LOGNAME too.
Here's an update which sets those, if they're present in libvirtd env.
The changed bit is here:
+ADD_ENV_COPY(LD_PRELOAD);
+ADD_ENV_COPY(LD_LIBRARY_PATH);
Looks good.
I guess you're adding those two in case qemu uses dlopen.
This time I've reviewed the rest of the patch.
Comments below:
+ADD_ENV_COPY(PATH);
+ADD_ENV_COPY(HOME);
+ADD_ENV_COPY(USER);
+ADD_ENV_COPY(LOGNAME);
+ADD_ENV_COPY(TMPDIR);
...
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
+#define ADD_ENV_SPACE \
...
+do {\
+if (qenvc == qenva) { \
+qenva += 10;\
+if (VIR_REALLOC_N(qenv, qenva) 0) \
+goto no_memory; \
+} \
+} while (0)
+
+#define ADD_ENV(thisarg)\
+do {\
+ADD_ENV_SPACE; \
+qenv[qenvc++] = thisarg;\
+} while (0)
+
+#define ADD_ENV_LIT(thisarg)\
+do {\
+ADD_ENV_SPACE; \
+if ((qenv[qenvc++] = strdup(thisarg)) == NULL) \
+goto no_memory; \
+} while (0)
+
+#define ADD_ENV_COPY(envname) \
+do {\
+char *val = getenv(envname);\
+ADD_ENV_SPACE; \
+if (val != NULL \
+(qenv[qenvc++] = strdup(val)) == NULL) \
+goto no_memory; \
+} while (0)
Doesn't this need to be adding envname=val strings,
rather than just val?
If it works as-is, then maybe none of these is actually used
(or at least they're not exercised in tests).
snprintf(memory, sizeof(memory), %lu, vm-def-memory/1024);
snprintf(vcpus, sizeof(vcpus), %lu, vm-def-vcpus);
+
+ADD_ENV_LIT(LC_ALL=C);
+
+ADD_ENV_COPY(LD_PRELOAD);
+ADD_ENV_COPY(LD_LIBRARY_PATH);
+ADD_ENV_COPY(PATH);
+ADD_ENV_COPY(HOME);
+ADD_ENV_COPY(USER);
+ADD_ENV_COPY(LOGNAME);
+ADD_ENV_COPY(TMPDIR);
emulator = vm-def-emulator;
...
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -22,11 +22,14 @@ static struct qemud_driver driver;
#define MAX_FILE 4096
-static int testCompareXMLToArgvFiles(const char *xml, const char *cmd, int
extraFlags) {
+static int testCompareXMLToArgvFiles(const char *xml,
...
+tmp = qenv;
+len = 0;
+while (*tmp) {
+if (actualargv[0])
+strcat(actualargv, );
+strcat(actualargv, *tmp);
+tmp++;
+}
tmp = argv;
len = 0;
No big deal, but both of those len = 0 assignments
can be removed, since len is no longer used.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Support SDL configuration for QEMU driver
On Wed, Oct 08, 2008 at 08:08:35PM +0100, Daniel P. Berrange wrote: On Thu, Oct 02, 2008 at 07:37:38PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: ... Now previously since we just use 'execv' the QEMU process would just inherit all libvirtd's environment variables. When we now use execve() no variables are inherited - we have to explicitly set all the ones we need. I'm not sure what we should consider the mimimum required? I'm merely setting 'LC_ALL=C' to ensure it runs in C locale. Do we need to set $PATH for QEMU - maybe ? Anything else which is good practice to set ? If QEMU uses PATH, then propagating that is necessary. I guess it's debatable whether to use PATH=$PATH or to use some hard-coded default on the RHS. But using PATH=$PATH seems friendlier, in case whatever QEMU uses is in some non-default location. If it uses mkstemp or the like, then including TMPDIR would be good. Depending on QEMU, maybe things like HOME, USER, LOGNAME too. Here's an update which sets those, if they're present in libvirtd env. The changed bit is here: +ADD_ENV_COPY(LD_PRELOAD); +ADD_ENV_COPY(LD_LIBRARY_PATH); +ADD_ENV_COPY(PATH); +ADD_ENV_COPY(HOME); +ADD_ENV_COPY(USER); +ADD_ENV_COPY(LOGNAME); +ADD_ENV_COPY(TMPDIR); +1 again, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Unify most error reporting (ver 2)
On Wed, Oct 08, 2008 at 08:35:38PM +0100, Daniel P. Berrange wrote:
One side question: is src/xmlrpc.{c,h} even used? And
if not, can it be removed?
It is not used at this time. It was for the hypothetical
alternative Xen driver which talks XML-RPC which we never
got around to writing. Perhaps it really is time to kill
it -we can get it back from CVS if ever required.
Agreed,
ACK to this patch
+1 too,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
[EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Xen and out of memory stuff, blocks libvirtd
On Thu, Oct 09, 2008 at 03:32:30AM +0200, Stefan de Konink wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Daniel P. Berrange schreef: On Wed, Oct 08, 2008 at 09:29:29PM +0200, Stefan de Konink wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Daniel P. Berrange schreef: This is nothing todo with XenD - the error shows fork() failed due to lack of OS memory. Whether running XenD or not, if the kernel runs out of memory you're doomed. You need more RAM allocated to your host. If XenD eats 90% of the available memory (at that time 256MB) I am seriously doomed aswell. Whatever the cause, its not a libvirt bug. If you think its wrong that XenD is eating 90% of your ram, then speak to the Xen developers - we can't help fix XenD here. Yes you can by providing a great technology preview for the alternate technique where XenD is not required anymore. Again that's not something we're doing in the context of libvirt project. Gerd is working on this with upstream QEMU community. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Hi, Not greatly familiar with this subject, but trying to follow your logic ... On Wed, 2008-10-08 at 10:51 -0500, Anthony Liguori wrote: Daniel P. Berrange wrote: - It is unsafe on host OS crash - all unflushed guest I/O will be lost, and there's no ordering guarentees, so metadata updates could be flushe to disk, while the journal updates were not. Say goodbye to your filesystem. This has nothing to do with cache=off. The IDE device defaults to write-back caching. As such, IDE makes no guarantee that when a data write completes, it's actually completed on disk. This only comes into play when write-back is disabled. I'm perfectly happy to accept a patch that adds explicit sync's when write-back is disabled. i.e. with write-back caching enabled, the IDE protocol makes no guarantees about when data is committed to disk. So, from a protocol correctness POV, qemu is behaving correctly with cache=on and write-back caching enabled on the disk. For SCSI, an unordered queue is advertised. Again, everything depends on whether or not write-back caching is enabled or not. Again, perfectly happy to take patches here. Queue ordering and write-back caching sound like very different things. Are they two distinct SCSI options, or ...? Surely an ordered queue doesn't do much help prevent fs corruption if the host crashes, right? You would still need write-back caching disabled? More importantly, the most common journaled filesystem, ext3, does not enable write barriers by default (even for journal updates). This is how it ship in Red Hat distros. i.e. implementing barriers for virtio won't help most ext3 deployments? And again, if barriers are just about ordering, don't you need to disable caching anyway? So there is no greater risk of corrupting a journal in QEMU than there is on bare metal. This is the bit I really don't buy - we're equating qemu caching to IDE write-back caching and saying the risk of corruption is the same in both cases. But doesn't qemu cache data for far, far longer than a typical IDE disk with write-back caching would do? Doesn't that mean you're far, far more likely to see fs corruption with qemu caching? Or put it another way, if we fix it by implementing the disabling of write-back caching ... users running a virtual machine will need to run hdparam -W 0 /dev/sda where they would never have run it on baremetal? Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Steve Ofsthun wrote: Anthony Liguori wrote: Daniel P. Berrange wrote: On Wed, Oct 08, 2008 at 11:06:27AM -0500, Anthony Liguori wrote: Sorry, it was mistakenly private - fixed now. Xen does use O_DIRECT for paravirt driver case - blktap is using the combo of AIO+O_DIRECT. You have to use O_DIRECT with linux-aio. And blktap is well known to have terrible performance. Most serious users use blkback/blkfront and blkback does not avoid the host page cache. It maintains data integrity by passing through barriers from the guest to the host. You can approximate this in userspace by using fdatasync. This is not accurate (at least for HVM guests using PV drivers on Xen 3.2). blkback does indeed bypass the host page cache completely. It's I/O behavior is akin to O_DIRECT. I reread the code more closely and convinced myself that you are correct. While it was obvious that the bio's were being constructed from granted pages, my initial impression was that the requests were still going through the scheduler and could still be satisfied from the host page cache. But that is not that case. I/O is dma'd directly to/from guest pages without involving any dom0 buffering. blkback barrier support only enforces write ordering of the blkback I/O stream(s). It does nothing to synchronize data in the host page cache. Data written through blkback will modify the storage underneath any data in the host page cache (w/o flushing the page cache). Subsequent access to the page cache by qemu-dm will access stale data. In our own Xen product we must explicitly flush the host page cache backing store data at qemu-dm start up, to guarantee proper data access. It is not safe to access the same backing object with both qemu-dm and blkback simultaneously. The issue the bug addresses, iozone performs better than native, can be addressed in the following way: 1) For IDE, you have to disable write-caching in the guest. This should force an fdatasync in the host. 2) For virtio-blk, we need to implement barrier support. This is what blkfront/blkback do. I don't think this is enough. Barrier semantics are local to a particular I/O stream. There would be no reason for the barrier to affect the host page cache (unless the I/Os are buffered by the cache). If we implement barriers in terms of fdatasync, it should be sufficient. Regards, Anthony Liguori -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cleanup virDomainCreateLinux into virDomainDefineXML
As promised in the libvirt-qpid thread, virDomainCreateLinux() call
name makes no sense (anymore), and it should be renamed, i guess since
virDomainCreate() already exist and by similarity with
virDomainDefineXML() the best name is virDomainCreateXML().
The associated patch rename virDomainCreateLinux to virDomainCreateXML
create a small function virDomainCreateLinux calling the former,
document it as deprecated. To help comprehension of the source code
it's also best to rename the internal driver method in a similar
way, which inflates the patch a bit but is IMHO worth it.
The patch also fixes a few #elif define(__sun__) and turn them
into the correct #elif defined(__sun__) cpp instructions, and changes
include/libvirt/virterror.h to improve the generated HTML page about the
deprected fileds in the structure, since we need to regenerate the
API xml and the docs, it was a good opportunity for that small change.
I removed the docs/ subdir part from the patch as it's generated
and mostly unreadable, that keeps it smaller too.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
[EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Index: include/libvirt/libvirt.h
===
RCS file: /data/cvs/libxen/include/libvirt/libvirt.h,v
retrieving revision 1.83
diff -u -p -r1.83 libvirt.h
--- include/libvirt/libvirt.h 23 Sep 2008 20:48:49 - 1.83
+++ include/libvirt/libvirt.h 9 Oct 2008 14:47:53 -
@@ -437,7 +437,7 @@ virConnectPtr virDomainGetConn
/*
* Domain creation and destruction
*/
-virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
+virDomainPtrvirDomainCreateXML (virConnectPtr conn,
const char *xmlDesc,
unsigned int flags);
virDomainPtrvirDomainLookupByName (virConnectPtr conn,
@@ -987,6 +987,12 @@ char * virStorageVolGet
char * virStorageVolGetPath(virStorageVolPtr vol);
+/*
+ * Deprecated calls
+ */
+virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
#ifdef __cplusplus
}
#endif
Index: include/libvirt/libvirt.h.in
===
RCS file: /data/cvs/libxen/include/libvirt/libvirt.h.in,v
retrieving revision 1.54
diff -u -p -r1.54 libvirt.h.in
--- include/libvirt/libvirt.h.in17 Sep 2008 14:15:21 - 1.54
+++ include/libvirt/libvirt.h.in9 Oct 2008 14:47:53 -
@@ -437,7 +437,7 @@ virConnectPtr virDomainGetConn
/*
* Domain creation and destruction
*/
-virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
+virDomainPtrvirDomainCreateXML (virConnectPtr conn,
const char *xmlDesc,
unsigned int flags);
virDomainPtrvirDomainLookupByName (virConnectPtr conn,
@@ -987,6 +987,12 @@ char * virStorageVolGet
char * virStorageVolGetPath(virStorageVolPtr vol);
+/*
+ * Deprecated calls
+ */
+virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
#ifdef __cplusplus
}
#endif
Index: include/libvirt/virterror.h
===
RCS file: /data/cvs/libxen/include/libvirt/virterror.h,v
retrieving revision 1.40
diff -u -p -r1.40 virterror.h
--- include/libvirt/virterror.h 11 Jul 2008 16:23:36 - 1.40
+++ include/libvirt/virterror.h 9 Oct 2008 14:47:53 -
@@ -78,15 +78,17 @@ struct _virError {
intdomain; /* What part of the library raised this error */
char *message;/* human-readable informative error message */
virErrorLevel level;/* how consequent is the error */
-virConnectPtr conn VIR_DEPRECATED; /* connection if available,
+virConnectPtr conn VIR_DEPRECATED; /* connection if available, deprecated
see note above */
-virDomainPtr dom VIR_DEPRECATED; /* domain if available, see note above */
+virDomainPtr dom VIR_DEPRECATED; /* domain if available, deprecated
+see note above */
char *str1; /* extra string information */
char *str2; /* extra string information */
char *str3; /* extra string information */
intint1; /* extra number information */
intint2;
[libvirt] [PATCH 0/3] Domain events - overview
The following patch series implements the Events API discussed here previously in this thread: http://www.redhat.com/archives/libvir-list/2008-September/msg00321.html This is a rather large patch, and touches quite a few files, but in the end, accomplishes the goal of event delivery of notificaton of domain events. It does so in a manner that is able to co-exist with older clients, or older libvirtd implementations. In the former case, events will never be delivered to a client that does not request them. In the latter case, we will fail registration, and so events will not be available. I have broken this up into 3 patches in the following manner: 1/3 - events.patch This is the bulk of the work. It emits, and delivers events to clients that are supported, and have registered to recieve the events. 2/3 - events-xen.patch These changes are currently untested (as I did my development with KVM), but are the changes that will be necessary to monitor xenstore for changes in domain states 3/3 - events-test.patch This is a test harness implemented to receive (and print) when domain events occur. I was unsure where this should live in the tree - so it currently is an independant patch (.c and Makefile in its own dir) Please note that I have not yet made any effort to generate any Python, or Java bindings, but wanted to get this out on the list first. NOTES: Thread safeness: We know that there are 2 data structures that need protection against concurrent access _virDriver.conns and _virConnect.domainEventCallbacks However, we have not addressed these issues at this juncture. We are considering recursive mutexes (specifically making conn-lock one) to avoid deadlocks in a vir* call from within a callback. We plan on addressing this with the next version of this code, after addressing any other issues the list may come up with, as well. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Domain events - xen
Monitor xenstore for domain changes
NOTE: this patch is currently untested
Signed-off-by: Ben Guthro [EMAIL PROTECTED]
proxy/Makefile.am |1
src/xen_unified.c |8 ++
src/xen_unified.h |3
src/xs_internal.c | 176 ++
src/xs_internal.h | 39 +++
5 files changed, 227 insertions(+)
diff --git a/proxy/Makefile.am b/proxy/Makefile.am
index 5902cab..75ba6b4 100644
--- a/proxy/Makefile.am
+++ b/proxy/Makefile.am
@@ -17,6 +17,7 @@ libvirt_proxy_SOURCES = libvirt_proxy.c @top_srcdir@/src/xend_internal.c \
@top_srcdir@/src/memory.c \
@top_srcdir@/src/domain_conf.c \
@top_srcdir@/src/util.c \
+ @top_srcdir@/src/event.c \
@top_srcdir@/src/uuid.c
libvirt_proxy_LDFLAGS = $(WARN_CFLAGS)
libvirt_proxy_DEPENDENCIES =
diff --git a/src/xen_unified.c b/src/xen_unified.c
index dae68d3..4ebf347 100644
--- a/src/xen_unified.c
+++ b/src/xen_unified.c
@@ -1294,6 +1294,12 @@ xenUnifiedNodeGetFreeMemory (virConnectPtr conn)
return(0);
}
+static int
+xenUnifiedDomainEventEmitted (virDomainEventType evt)
+{
+return xenStoreDomainEventEmitted(evt);
+}
+
/*- Register with libvirt.c, and initialise Xen drivers. -*/
#define HV_VERSION ((DOM0_INTERFACE_VERSION 24) * 100 + \
@@ -1305,6 +1311,7 @@ static virDriver xenUnifiedDriver = {
.no = VIR_DRV_XEN_UNIFIED,
.name = Xen,
.ver = HV_VERSION,
+.conns = NULL,
.probe = xenUnifiedProbe,
.open = xenUnifiedOpen,
.close = xenUnifiedClose,
@@ -1359,6 +1366,7 @@ static virDriver xenUnifiedDriver = {
.domainBlockPeek = xenUnifiedDomainBlockPeek,
.nodeGetCellsFreeMemory = xenUnifiedNodeGetCellsFreeMemory,
.getFreeMemory = xenUnifiedNodeGetFreeMemory,
+.domainEventEmitted = xenUnifiedDomainEventEmitted,
};
/**
diff --git a/src/xen_unified.h b/src/xen_unified.h
index c17b498..e4e7a59 100644
--- a/src/xen_unified.h
+++ b/src/xen_unified.h
@@ -12,6 +12,7 @@
#define __VIR_XEN_UNIFIED_H__
#include internal.h
+#include xs_internal.h
#include capabilities.h
#ifndef HAVE_WINSOCK2_H
@@ -110,6 +111,8 @@ struct _xenUnifiedPrivate {
* xen_unified.c.
*/
int opened[XEN_UNIFIED_NR_DRIVERS];
+
+xenStoreWatchListPtr xsWatchHead;
};
typedef struct _xenUnifiedPrivate *xenUnifiedPrivatePtr;
diff --git a/src/xs_internal.c b/src/xs_internal.c
index 316604a..e60549b 100644
--- a/src/xs_internal.c
+++ b/src/xs_internal.c
@@ -17,6 +17,7 @@
#include fcntl.h
#include sys/mman.h
#include sys/ioctl.h
+#include sys/poll.h
#include stdint.h
@@ -28,6 +29,8 @@
#include internal.h
#include driver.h
+#include memory.h
+#include event.h
#include xen_unified.h
#include xs_internal.h
#include xen_internal.h /* for xenHypervisorCheckID */
@@ -40,6 +43,9 @@
#error unsupported platform
#endif
+#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt,__VA_ARGS__)
+#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
+
#ifndef PROXY
static char *xenStoreDomainGetOSType(virDomainPtr domain);
@@ -319,6 +325,19 @@ xenStoreOpen(virConnectPtr conn,
}
return (-1);
}
+
+/* Register for domain watch callbacks */
+xenStoreAddWatch(priv-xsWatchHead, @introduceDomain,
+ introduceDomain, xenStoreDomainIntroduced, priv);
+xenStoreAddWatch(priv-xsWatchHead, @releaseDomain,
+ releaseDomain, xenStoreDomainReleased, priv);
+/* Add an event handle */
+if (virEventAddHandle(xs_fileno(priv-xshandle),
+ POLLIN | POLLPRI,
+ xenStoreWatchEvent,
+ conn) 0) {
+return (-1);
+}
return (0);
}
@@ -344,6 +363,7 @@ xenStoreClose(virConnectPtr conn)
if (priv-xshandle == NULL)
return(-1);
+xenStoreWatchListFree(priv-xsWatchHead);
xs_daemon_close(priv-xshandle);
return (0);
}
@@ -937,3 +957,159 @@ char *xenStoreDomainGetName(virConnectPtr conn,
return xs_read(priv-xshandle, 0, prop, len);
}
+void xenStoreWatchListFree(xenStoreWatchListPtr head)
+{
+while (head) {
+xenStoreWatchListPtr p = head-next;
+VIR_FREE(head-path);
+VIR_FREE(head-token);
+VIR_FREE(head);
+head = p;
+}
+}
+
+int xenStoreAddWatch(xenStoreWatchListPtr head,
+ const char *path,
+ const char *token,
+ xenStoreWatchCallback cb,
+ void *opaque)
+{
+xenStoreWatchListPtr watch;
+xenStoreWatchListPtr p = head;
+
+if (VIR_ALLOC(watch) 0)
+return -1;
+watch-path = strdup(path);
+watch-token = strdup(token);
+watch-cb = cb;
+watch-opaque = opaque;
+
+/* find end of list */
+while(p) p = p-next;
+
+DEBUG(Added xs watch %s %s, path, token);
+p = watch;
+return 0;
+}
+
+int
Re: [libvirt] [PATCH 3/3] Domain events - test harness
This test app prints domain events as they are emitted.
I was not sure where it belonged in the tree - so this is not in the context of
the rest of the libvirt tree
Signed-off-by: Ben Guthro [EMAIL PROTECTED]
Makefile |2
event-test.c | 134 +++
2 files changed, 136 insertions(+)
--- /dev/null 2008-09-18 12:09:06 -04:00
+++ ./event-test.c 2008-10-03 14:31:49 -04:00
@@ -0,0 +1,134 @@
+#include stdio.h
+#include sys/types.h
+#include sys/poll.h
+
+#include libvirt/libvirt.h
+
+#define DEBUG0(fmt) printf(%s:%d :: fmt \n, __FUNCTION__, __LINE__)
+#define DEBUG(fmt, ...) printf(%s:%d: fmt \n, __FUNCTION__, __LINE__, __VA_ARGS__)
+
+int g_fd = 0;
+int g_event = 0;
+virEventHandleCallback g_cb = NULL;
+void *g_opaque = NULL;
+#define TIMEOUT_MS 1000
+
+int myDomainEventCallback (virConnectPtr conn,
+virDomainPtr dom,
+int event,
+void *opaque)
+{
+printf(EVENT: Domain %s(%d) , virDomainGetName(dom), virDomainGetID(dom));
+switch(event) {
+case VIR_DOMAIN_EVENT_ADDED:
+printf(Added);
+break;
+case VIR_DOMAIN_EVENT_REMOVED:
+printf(Removed);
+break;
+case VIR_DOMAIN_EVENT_STARTED:
+printf(Started);
+break;
+case VIR_DOMAIN_EVENT_SUSPENDED:
+printf(Suspended);
+break;
+case VIR_DOMAIN_EVENT_RESUMED:
+printf(Resumed);
+break;
+case VIR_DOMAIN_EVENT_STOPPED:
+printf(Stopped);
+break;
+case VIR_DOMAIN_EVENT_SAVED:
+printf(Saved);
+break;
+case VIR_DOMAIN_EVENT_RESTORED:
+printf(Restored);
+break;
+default:
+printf(Unknown Event);
+}
+printf(\n);
+return 0;
+}
+
+int myEventAddHandleFunc(int fd, int event, virEventHandleCallback cb, void *opaque)
+{
+DEBUG(Add handle %d %d %p %p, fd, event, cb, opaque);
+g_fd = fd;
+g_event = event;
+g_cb = cb;
+g_opaque = opaque;
+return 0;
+}
+void myEventUpdateHandleFunc(int fd, int event)
+{
+DEBUG(Updated Handle %d %d, fd, event);
+return;
+}
+int myEventRemoveHandleFunc(int fd)
+{
+DEBUG(Removed Handle %d, fd);
+return 0;
+}
+
+void virEventRegisterHandleImpl(virEventAddHandleFunc addHandle,
+virEventUpdateHandleFunc updateHandle,
+virEventRemoveHandleFunc removeHandle);
+
+void usage(const char *pname)
+{
+printf(%s uri\n, pname);
+}
+int main(int argc, char **argv)
+{
+int run=1;
+int sts;
+
+if(argv[1] strcmp(argv[1],--help)==0) {
+usage(argv[0]);
+return -1;
+}
+virEventRegisterHandleImpl( myEventAddHandleFunc,
+myEventUpdateHandleFunc,
+myEventRemoveHandleFunc);
+virConnectPtr dconn = NULL;
+dconn = virConnectOpen (argv[1] ? argv[1] : qemu:///system);
+if (!dconn) {
+printf(error opening\n);
+return -1;
+}
+
+DEBUG0(Registering domain event cb);
+virConnectDomainEventRegister(dconn, myDomainEventCallback, NULL);
+
+while(run) {
+struct pollfd pfd = { .fd = g_fd,
+ .events = g_event,
+ .revents = 0};
+
+sts = poll(pfd, 1, TIMEOUT_MS);
+if (sts == 0) {
+/* DEBUG0(Poll timeout); */
+continue;
+}
+if (sts 0 ) {
+DEBUG0(Poll failed);
+continue;
+}
+if ( pfd.revents POLLHUP ) {
+DEBUG0(Reset by peer);
+return -1;
+}
+
+DEBUG(sts = %d, sts);
+DEBUG(Calling CB: %p (%d,%d,%p), g_cb, g_fd, g_event, g_opaque);
+g_cb(g_fd, g_event, g_opaque);
+
+}
+
+if( dconn virConnectClose(dconn)0 ) {
+printf(error closing\n);
+}
+printf(done\n);
+}
+
--- /dev/null 2008-09-18 12:09:06 -04:00
+++ ./Makefile 2008-09-30 12:56:44 -04:00
@@ -0,0 +1,2 @@
+LDFLAGS += -lvirt
+event-test: event-test.c
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Libvirt.org will be down in an hour till tomorrow morning
There is major electric change at the hosting place for libvirt.org and the server will need to be shutdown for the night (french time, i.e. beginning of the afternoon US EST time). Hopefully the services will be restarted tomorrow morning, sorry for the disruption and the late announcement, I just learned about it... Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Unify most error reporting (ver 2)
Daniel Veillard wrote:
On Wed, Oct 08, 2008 at 08:35:38PM +0100, Daniel P. Berrange wrote:
One side question: is src/xmlrpc.{c,h} even used? And
if not, can it be removed?
It is not used at this time. It was for the hypothetical
alternative Xen driver which talks XML-RPC which we never
got around to writing. Perhaps it really is time to kill
it -we can get it back from CVS if ever required.
Agreed,
ACK to this patch
+1 too,
Daniel
Thanks, pushed now.
- Cole
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
* Anthony Liguori ([EMAIL PROTECTED]) wrote: Mark McLoughlin wrote: This is the bit I really don't buy - we're equating qemu caching to IDE write-back caching and saying the risk of corruption is the same in both cases. Yes. I'm with Mark here. But doesn't qemu cache data for far, far longer than a typical IDE disk with write-back caching would do? Doesn't that mean you're far, far more likely to see fs corruption with qemu caching? It caches more data, I don't know how much longer it cases than a typical IDE disk. The guest can crash and that won't cause data loss. The only thing that will really cause data loss is the host crashing so it's slightly better than write-back caching from that regard. Or put it another way, if we fix it by implementing the disabling of write-back caching ... users running a virtual machine will need to run hdparam -W 0 /dev/sda where they would never have run it on baremetal? I don't see it as something needing to be fixed because I don't see that the exposure is significantly greater for a VM than for a real machine. One host crash corrupting all VM's data? What is the benefit? Seems like the benefit of caching is only useful when VM's aren't all that busy. Once the host is heavily committed, the case where it might benefit most from the extra caching, the host cache will shrink to essentially nothing. Also, many folks will be running heterogenous guests (or at least not template based), so in that case it's really just double caching (i.e. memory overhead). Seems a no-brainer to me, so I must be confused and/or missing smth. thanks, -chris -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
Hi Daniel -
I'm implementing virNodeDeviceDef, as suggested, but I just noticed
something that really bothers me. In a nutshell, it's that the current
implementation of this scheme unnecessarily increases the (time)
complexity of O(1) operations to O(n).
For example, take virDomainGetXMLDesc(). One would think dumping the
XML descriptor for a domain object we have in hand would be O(1), but in
fact, it's O(n) (where n is the number of domains with the same driver),
because qemudDomainDumpXML must find the DomainObjPtr (from which it can
get the def). I suppose this lookup could be made O(log n) if the
domains were sorted by UUID on the list, but the fact remains that
lookup could be eliminated entirely.
Perhaps it's just a matter of allowing the public objects
(virDomain, virNetwork, etc.) to hold a pointer to their optional (never
used by remote driver, for example) private state objects. So I guess
I'm suggesting adding a void * to each of the public objects, which
drivers can use for this purpose. I'll go ahead and do this for
NodeDevices in the next incarnation of this patch to show you what I
mean. (It won't be hard go to the conventional lookup impl if this
turns out to be a bad idea.)
Dave
On Fri, 2008-10-03 at 16:23 +0100, Daniel P. Berrange wrote:
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
diff --git a/src/internal.h b/src/internal.h
index d96504d..9a94f6d 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -292,6 +307,25 @@ struct _virStorageVol {
};
+/**
+ * _virNodeDevice:
+ *
+ * Internal structure associated with a node device
+ */
+struct _virNodeDevice {
+unsigned int magic; /* specific value to check */
+int refs; /* reference count */
+virConnectPtr conn; /* pointer back to the connection
*/
+char *key; /* device key */
+char *name; /* device name */
+char *parent_key; /* parent device */
+char *bus_name; /* (optional) bus name */
+xmlNodePtr bus; /* (optional) bus descriptor */
+char **cap_names; /* capability names (null-term) */
+xmlNodePtr *caps; /* capability descs (null-term) */
+};
This is not the right way to maintain the internal representation of
devices.
The model we follow is to have 2, sometimes 3 structs to represent
an object. I'll summarize in terms of domain objects
- virDomainPtr - the public opaque struct representing a domain
the internal struct is defined in internal.h
and merely contains reference counting, connection
pointer, and any unique keys (ID, name, UUID).
- virDomainObjPtr - the internal struct representing the state of
a domain object. Not all drivers use us - only
stateful drivers do. It is used to track state
such as guest PID, logfile, running state,
and a pointer to the live and inactive configs
This is done in domain_conf.h/.c
- virDomainDefPtr - the internal struct representing the canonical
configuration of a domain. This is a direct
mapping of the XML into a series of structs.
This is done in domain_conf.h/.c
So, in the context for host devices we need a few changes. In the
internal.h file, you should only store the key/name attributes.
There should then be a separate file handling configuration parsing
and formatting, node_device_conf.h/.c. There is probably no need
for a state object, so skip virNodeDeviceObjPtr, and just create a
virNodeDeviceDefPtr representing the XML format as a series of
structs/unions, etc. See virDomainDefPtr for a good example. This
should not store any xmlNodePtr instances - it should be all done
as explicit struct/enum fields
The node_device_conf.c file should at mimium have 2 methods, one
for converting an XML document into a virNodeDeviceDefPtr object,
and one for the converting a virNodeDeviceObjPtr back into an XML
document. Follow the existing API naming / method signatures that
are seen in domain_conf.h / network_conf.h for current 'best practice'
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, Oct 09, 2008 at 12:25:18PM -0400, David Lively wrote:
Hi Daniel -
I'm implementing virNodeDeviceDef, as suggested, but I just noticed
something that really bothers me. In a nutshell, it's that the current
implementation of this scheme unnecessarily increases the (time)
complexity of O(1) operations to O(n).
For example, take virDomainGetXMLDesc(). One would think dumping the
XML descriptor for a domain object we have in hand would be O(1), but in
fact, it's O(n) (where n is the number of domains with the same driver),
because qemudDomainDumpXML must find the DomainObjPtr (from which it can
get the def). I suppose this lookup could be made O(log n) if the
domains were sorted by UUID on the list, but the fact remains that
lookup could be eliminated entirely.
Sure the lookup algorithm is O(n), but have you actually got real
world benchmarks showing this is the serious bottleneck? In the XML
dump example, the time to lookup the object is likely dwarfed by the
time to generate the XML doc, or to talk to the QEMU monitor.
IMHO, optimizing this is overkill. Worst case we can just hash on
the UUID if it ever provdes a problem.
Perhaps it's just a matter of allowing the public objects
(virDomain, virNetwork, etc.) to hold a pointer to their optional (never
used by remote driver, for example) private state objects. So I guess
I'm suggesting adding a void * to each of the public objects, which
drivers can use for this purpose. I'll go ahead and do this for
NodeDevices in the next incarnation of this patch to show you what I
mean. (It won't be hard go to the conventional lookup impl if this
turns out to be a bad idea.)
No please don't do that. We really need to keep the hard separation
between the public vir{Domain,Network,Storage,Node}Ptr objects, and
the internal objects completely separated. The only association
should be via the various unique keys - ID, Name UUID as appropriate.
The public objects do not neccessarily have the same lifecycle as
the internal object - eg, an app can hold onto the public object
even after the internal representation has been killed off.
Keeping a reference from the public to private objects, means we also
need to have a reverse references from priate to public objects, so we
can kill off the references when objects go away. This also means we
need to have complex locking from the internal drivers to the public
objects which for thread safety which I really do not want.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
Chris Wright wrote: * Anthony Liguori ([EMAIL PROTECTED]) wrote: Mark McLoughlin wrote: This is the bit I really don't buy - we're equating qemu caching to IDE write-back caching and saying the risk of corruption is the same in both cases. Yes. I'm with Mark here. I've been persuaded. Relying on the host's integrity for guest data integrity is not a good idea by default. I don't think we should use cache=off to address this though. I've sent a patch and started a thread on qemu-devel. Let's continue the conversation there. Regards, Anthony Liguori -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Disable QEMU drive caching
* Anthony Liguori ([EMAIL PROTECTED]) wrote: I've been persuaded. Relying on the host's integrity for guest data integrity is not a good idea by default. I don't think we should use cache=off to address this though. I've sent a patch and started a thread on qemu-devel. Let's continue the conversation there. Thanks, both for the patch, and redirecting conversation to proper spot. -chris -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Domain events - primary implementation
On Thu, Oct 09, 2008 at 11:22:38AM -0400, Ben Guthro wrote:
This patch is somewhat large, and touches many files.
For large patches like this, its useful to split it up
in order of:
- The public header API libvirt.c/driver.c glue layer
- The daemon code
- One patch per internal driver
I hope the review that follows doesn't come off too negative - its
basically all just hinging around one core point. Namely that the
tracking dispatch of callbacks needs to be pushed further down
into the drivers themselves. This is to avoid the driver having to
keep so many references back to the public facing objects state
which rather tangles things up.
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index d519452..9b6e1da 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -987,6 +987,50 @@ char * virStorageVolGetXMLDesc
(virStorageVolPtr pool,
char * virStorageVolGetPath(virStorageVolPtr
vol);
+/*
+ * Domain Event Notification
+ */
+
+typedef enum {
+ VIR_DOMAIN_EVENT_ADDED,
+ VIR_DOMAIN_EVENT_REMOVED,
+ VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_SAVED,
+ VIR_DOMAIN_EVENT_RESTORED,
+} virDomainEventType;
+
+typedef int (*virConnectDomainEventCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ int event,
+ void *opaque);
+
+int virConnectDomainEventRegister(virConnectPtr conn,
+ virConnectDomainEventCallback cb,
+ void *opaque);
+
+int virConnectDomainEventDeregister(virConnectPtr conn,
+virConnectDomainEventCallback cb);
+
+/**
+ * virEventHandleCallback: callback for receiving file handle events
+ *
+ * @fd: file handle on which the event occurred
+ * @events: bitset of events from POLLnnn constants
We probably want to explicit define these constants in our header
file, since now this is public we need it to be portable, regardless
of the internal impl.
diff --git a/qemud/qemud.h b/qemud/qemud.h
index 91cb939..63784cd 100644
--- a/qemud/qemud.h
+++ b/qemud/qemud.h
@@ -132,6 +132,10 @@ struct qemud_client {
*/
virConnectPtr conn;
+/* This client supports events, and has registered for at least
+ one event type. This is a bitmask of requested event types */
+int events_registered;
I'm not sure why the daemon needs to know this. I'd expect
that it just gets a RPC call for each virConnectDomainEventRegister()
and virConnectDomainEventDeregister() invocation, and pass these
straight into the 'virConnectPtr' it has, not register one global
callback and try to filter events itself.
+static int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainPtr dom,
+ int event,
+ void *opaque)
+{
+struct qemud_server *server = opaque;
+REMOTE_DEBUG(Relaying domain event %d, event);
+
+struct qemud_client *c = server-clients;
+while(c) {
+if ( c-conn == conn
+ (c-events_registered virDomainEvent) ) {
This check shoudln't be needed if you're only registering the
callbacks when the client actually asks for them.
+remoteDispatchDomainEventSend (c, dom, event);
+qemudDispatchClientWrite(server,c);
+} else {
+REMOTE_DEBUG(Event class %d not registered for client,
virDomainEvent);
+}
+c = c-next;
+}
+return 0;
+}
@@ -436,6 +469,11 @@ remoteDispatchOpen (struct qemud_server *server
ATTRIBUTE_UNUSED,
? virConnectOpenReadOnly (name)
: virConnectOpen (name);
+/* Register event delivery callback */
+if(client-conn) {
+REMOTE_DEBUG(%s,Registering to relay remote events);
+virConnectDomainEventRegister(client-conn, remoteRelayDomainEvent,
server);
+}
return client-conn ? 0 : -1;
}
Hence this shouldn't be needed at time of open.
+
+/***
+ * Enabe / disable event classes
+ ***/
+static int remoteDispatchEventsEnable (struct qemud_server *server
ATTRIBUTE_UNUSED,
+ struct qemud_client *client,
+ remote_message_header *req
ATTRIBUTE_UNUSED,
+ remote_events_enable_args *args,
+ remote_events_enable_ret *ret)
+{
+CHECK_CONN(client);
+if(args-enable) {
+client-events_registered |= args-event_class;
+} else {
+client-events_registered =
Re: [libvirt] [PATCH 2/3] Domain events - xen
On Thu, Oct 09, 2008 at 11:23:16AM -0400, Ben Guthro wrote: Monitor xenstore for domain changes NOTE: this patch is currently untested Nice that this is so simple for getting destroy/start events. Its a shame xen doesn't store the paused/crash/etc state here too though. I guess we'll have to try elsewhere if we want to expose that - XenAPI is probably the only viable option there. IIRC XenAPI provided a way to make an XMLRPC call which blocked until a state change event arrived which is the only way to avoid polling with XMLRPC Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Domain events - test harness
On Thu, Oct 09, 2008 at 11:24:47AM -0400, Ben Guthro wrote: This test app prints domain events as they are emitted. I was not sure where it belonged in the tree - so this is not in the context of the rest of the libvirt tree Since it relies on host OS state, its not relaly something we can reliably integrate into the unit tests. So perhaps we should have a 'demos' directory to put this in. Its certainly useful for both us debugging the drivers, and for developers wanting a simple example of using events. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, 2008-10-09 at 18:01 +0100, Daniel P. Berrange wrote: On Thu, Oct 09, 2008 at 12:25:18PM -0400, David Lively wrote: Hi Daniel - I'm implementing virNodeDeviceDef, as suggested, but I just noticed something that really bothers me. In a nutshell, it's that the current implementation of this scheme unnecessarily increases the (time) complexity of O(1) operations to O(n). For example, take virDomainGetXMLDesc(). One would think dumping the XML descriptor for a domain object we have in hand would be O(1), but in fact, it's O(n) (where n is the number of domains with the same driver), because qemudDomainDumpXML must find the DomainObjPtr (from which it can get the def). I suppose this lookup could be made O(log n) if the domains were sorted by UUID on the list, but the fact remains that lookup could be eliminated entirely. Sure the lookup algorithm is O(n), but have you actually got real world benchmarks showing this is the serious bottleneck? In the XML Well ... uhh ... no. :-o dump example, the time to lookup the object is likely dwarfed by the time to generate the XML doc, or to talk to the QEMU monitor. IMHO, optimizing this is overkill. Worst case we can just hash on the UUID if it ever provdes a problem. Okay, I'll buy that. And (now) I see what you mean about the differing object lifetime. ... In fact (perhaps I shouldn't be admitting this publicly), your whole public-obj/Obj scheme now finally makes sense to me! The public objs are really just a kind of handle to the private Objs, and they exist independently of one another. public objs belong to the apps, private Objs to the appropriate driver. And (now that I look at the Xen driver), I finally understand that there are stateless drivers. In fact, both the devkit and HAL node-dev drivers need to be stateful (since the device name isn't a reasonable handle for either devkit or HAL devices), so I'm creating a virNodeDeviceObj as well as a virNodeDeviceDef. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Domain events - primary implementation
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512
Daniel P. Berrange schreef:
+typedef enum {
+ VIR_DOMAIN_EVENT_ADDED,
+ VIR_DOMAIN_EVENT_REMOVED,
+ VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_SAVED,
+ VIR_DOMAIN_EVENT_RESTORED,
+} virDomainEventType;
Is it possible to get an *active* migration state event from the source.
In xm list the name seems to change to migrating-.
Stefan
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEAREKAAYFAkjuTmkACgkQYH1+F2Rqwn35vACaAjyvkk8A26xEpwRZErxUIYa+
jUsAn12ekFAZLNI9EQc9p1xvL0WdnHB4
=TsbX
-END PGP SIGNATURE-
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Domain events - primary implementation
On Thu, Oct 09, 2008 at 06:50:15PM +0100, Daniel P. Berrange wrote:
On Thu, Oct 09, 2008 at 11:22:38AM -0400, Ben Guthro wrote:
diff --git a/src/internal.h b/src/internal.h
index a3d48fa..67a3e5b 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -191,6 +191,18 @@ extern int debugFlag;
#define VIR_IS_STORAGE_VOL(obj)((obj)
(obj)-magic==VIR_STORAGE_VOL_MAGIC)
#define VIR_IS_CONNECTED_STORAGE_VOL(obj) (VIR_IS_STORAGE_VOL(obj)
VIR_IS_CONNECT((obj)-conn))
+/**
+ * Domain Event Callbacks
+ */
+struct _virDomainEventCallbackList {
+virConnectPtr conn;
+virConnectDomainEventCallback cb;
+void *opaque;
+struct _virDomainEventCallbackList *next;
+};
+
+typedef struct _virDomainEventCallbackList virDomainEventCallbackList;
+
/*
* arbitrary limitations
*/
@@ -237,6 +249,12 @@ struct _virConnect {
virHashTablePtr storagePools;/* hash table for known storage pools */
virHashTablePtr storageVols;/* hash table for known storage vols */
int refs; /* reference count */
+
+/* Domain Callbacks */
+virDomainEventCallbackList *domainEventCallbacks;
+
+/* link to next conn of this driver type */
+struct _virConnect *next;
};
This doesn't make any sense to me - you're making all the virConnect
objects cross reference each other. Each object only needs to know
about the callbacks registered to itself - it doesn't care about
callbacks other virConnect objects have. I'd expect something
more like
struct _virDomainEventCallback {
virConnectDomainEventCallback cb;
void *opaque;
};
typedef struct _virDomainEventCallback virDomainEventCallback;
typedef virDomainEventCallback *virDomainEventCallbackPtr;
struct _virDomainEventCallbackList {
unsigned int ndomainCallbacks;
struct __virDomainEventCallbackPtr *domainCallbacks;
}
typedef struct _virDomainEventCallbackList virDomainEventCallbackList;
I used a explicit array here, because we're trying to kill off the linked
lists, since it simplifies thread safety work, to be able to lock individual
objects, and not worry about its peers pointing to it - only its parent.
The internal drivers would then have virDomainEventCallbackList intances
as they need - eg, in the _xenUnifiedPrivate struct for Xen drivers, or
in the 'struct qemud_driver' for QEMU, etc.
Correcting myself here - the _virDomainEventCallback struct does
actually need to keep a copy of the virConnectPtr object. Since
if we're storing it in the individual drivers, that's the only
link back to the connection. It can be treated opaquely though
in this instance - merely another pointer to be fed back into
the callback. Which reminds me, when registering the callback
it will be neccessary to increment the ref count on the virconnect
object to make sure it won't be de-allocated while an event
calback is still present.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Domain events - primary implementation
On Thu, Oct 09, 2008 at 08:33:14PM +0200, Stefan de Konink wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512
Daniel P. Berrange schreef:
+typedef enum {
+ VIR_DOMAIN_EVENT_ADDED,
+ VIR_DOMAIN_EVENT_REMOVED,
+ VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_SAVED,
+ VIR_DOMAIN_EVENT_RESTORED,
+} virDomainEventType;
Is it possible to get an *active* migration state event from the source.
In xm list the name seems to change to migrating-.
These events are reflecting the result state of a VM. When a outgoing
migration starts, it is still running as far as everyone should be
concerned - its supposed to be totally transparent. Once it completes
the results state is 'removed'. For an incoming migration, a VM starts
in the paused state until its finished migrating, at which point it is
running. So all the state changes associated with migration are tracked
already.
I can see it might be desirable to be able to explicitly tell when an
outgoing migration starts completes, and when an incoming migration
starts completes. You'd likely want more info in the context fo the
migration event though - eg the destination host, and the source host.
So perhaps we should have an explicit migration event type you can
register for, separately from the domain execution lifecycle states.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, Oct 09, 2008 at 02:29:18PM -0400, David Lively wrote: On Thu, 2008-10-09 at 18:01 +0100, Daniel P. Berrange wrote: dump example, the time to lookup the object is likely dwarfed by the time to generate the XML doc, or to talk to the QEMU monitor. IMHO, optimizing this is overkill. Worst case we can just hash on the UUID if it ever provdes a problem. Okay, I'll buy that. And (now) I see what you mean about the differing object lifetime. ... In fact (perhaps I shouldn't be admitting this publicly), your whole public-obj/Obj scheme now finally makes sense to me! The public objs are really just a kind of handle to the private Objs, and they exist independently of one another. public objs belong to the apps, private Objs to the appropriate driver. And (now that I look at the Xen driver), I finally understand that there are stateless drivers. I think its clear that the major thing lacking here is good documentation on the structure / internals of libvirt its object/driver modelling. Until very recently it was pretty much every driver for itself. With the introduction of unified internal objects, we do now have some formal data structure, beyond just the driver.h API contracts. Until we document this its not surprising that people find it all rather opaque to understand Time for me to add a section to the website docs covering internal architecture in more detail... In fact, both the devkit and HAL node-dev drivers need to be stateful (since the device name isn't a reasonable handle for either devkit or HAL devices), so I'm creating a virNodeDeviceObj as well as a virNodeDeviceDef. Great, that meshes with my mental model of things Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt.org will be down in an hour till tomorrow morning
On Thu, Oct 09, 2008 at 05:42:07PM +0200, Daniel Veillard wrote: There is major electric change at the hosting place for libvirt.org and the server will need to be shutdown for the night (french time, i.e. beginning of the afternoon US EST time). Hopefully the services will be restarted tomorrow morning, sorry for the disruption and the late announcement, I just learned about it... FYI, if anyone urgently needs the latest source, the GIT mirror will remain online throughout the outage: http://git.et.redhat.com/?p=libvirt.git Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cleanup virDomainCreateLinux into virDomainDefineXML
On Thu, Oct 09, 2008 at 05:16:37PM +0200, Daniel Veillard wrote:
As promised in the libvirt-qpid thread, virDomainCreateLinux() call
name makes no sense (anymore), and it should be renamed, i guess since
virDomainCreate() already exist and by similarity with
virDomainDefineXML() the best name is virDomainCreateXML().
Agreed - we already have virNetworkCreateXML(), so virDomainCreateXML()
makes perfect sense.
The associated patch rename virDomainCreateLinux to virDomainCreateXML
create a small function virDomainCreateLinux calling the former,
document it as deprecated. To help comprehension of the source code
it's also best to rename the internal driver method in a similar
way, which inflates the patch a bit but is IMHO worth it.
Since this doesn't impact the on-the-wire RPC number, this seems
like a worthwhile change too.
The patch also fixes a few #elif define(__sun__) and turn them
into the correct #elif defined(__sun__) cpp instructions, and changes
include/libvirt/virterror.h to improve the generated HTML page about the
deprected fileds in the structure, since we need to regenerate the
API xml and the docs, it was a good opportunity for that small change.
I removed the docs/ subdir part from the patch as it's generated
and mostly unreadable, that keeps it smaller too.
ACK to this all with just a few minor nit-picks.
+/**
+ * virDomainCreateLinux:
+ * @conn: pointer to the hypervisor connection
+ * @xmlDesc: string containing an XML description of the domain
+ * @flags: callers should always pass 0
+ *
+ * Deprecated after 0.4.6 use virDomainCreateXML()
Can we make this a little clear that they provide 100% functionally
identical impl. Something like
* Deprecated after 0.4.6.
* Renamed to virDomainCreateXML() providing identical functionality.
* This existing name will left indefinitely for API compatability.
Index: qemud/remote_protocol.c
===
RCS file: /data/cvs/libxen/qemud/remote_protocol.c,v
retrieving revision 1.18
diff -u -p -r1.18 remote_protocol.c
--- qemud/remote_protocol.c 5 Sep 2008 12:03:45 - 1.18
+++ qemud/remote_protocol.c 9 Oct 2008 14:47:54 -
@@ -618,7 +618,7 @@ xdr_remote_num_of_domains_ret (XDR *xdrs
}
bool_t
-xdr_remote_domain_create_linux_args (XDR *xdrs,
remote_domain_create_linux_args *objp)
+xdr_remote_domain_create_xml_args (XDR *xdrs, remote_domain_create_xml_args
*objp)
{
if (!xdr_remote_nonnull_string (xdrs, objp-xml_desc))
@@ -629,7 +629,7 @@ xdr_remote_domain_create_linux_args (XDR
}
bool_t
-xdr_remote_domain_create_linux_ret (XDR *xdrs,
remote_domain_create_linux_ret *objp)
+xdr_remote_domain_create_xml_ret (XDR *xdrs, remote_domain_create_xml_ret
*objp)
{
if (!xdr_remote_nonnull_domain (xdrs, objp-dom))
Index: qemud/remote_protocol.h
===
RCS file: /data/cvs/libxen/qemud/remote_protocol.h,v
retrieving revision 1.18
diff -u -p -r1.18 remote_protocol.h
--- qemud/remote_protocol.h 5 Sep 2008 12:03:45 - 1.18
+++ qemud/remote_protocol.h 9 Oct 2008 14:47:54 -
@@ -318,16 +318,16 @@ struct remote_num_of_domains_ret {
};
typedef struct remote_num_of_domains_ret remote_num_of_domains_ret;
-struct remote_domain_create_linux_args {
+struct remote_domain_create_xml_args {
remote_nonnull_string xml_desc;
int flags;
};
-typedef struct remote_domain_create_linux_args
remote_domain_create_linux_args;
+typedef struct remote_domain_create_xml_args remote_domain_create_xml_args;
-struct remote_domain_create_linux_ret {
+struct remote_domain_create_xml_ret {
remote_nonnull_domain dom;
};
-typedef struct remote_domain_create_linux_ret remote_domain_create_linux_ret;
+typedef struct remote_domain_create_xml_ret remote_domain_create_xml_ret;
struct remote_domain_lookup_by_id_args {
int id;
@@ -1264,8 +1264,8 @@ extern bool_t xdr_remote_domain_memory_
extern bool_t xdr_remote_list_domains_args (XDR *,
remote_list_domains_args*);
extern bool_t xdr_remote_list_domains_ret (XDR *, remote_list_domains_ret*);
extern bool_t xdr_remote_num_of_domains_ret (XDR *,
remote_num_of_domains_ret*);
-extern bool_t xdr_remote_domain_create_linux_args (XDR *,
remote_domain_create_linux_args*);
-extern bool_t xdr_remote_domain_create_linux_ret (XDR *,
remote_domain_create_linux_ret*);
+extern bool_t xdr_remote_domain_create_xml_args (XDR *,
remote_domain_create_xml_args*);
+extern bool_t xdr_remote_domain_create_xml_ret (XDR *,
remote_domain_create_xml_ret*);
extern bool_t xdr_remote_domain_lookup_by_id_args (XDR *,
remote_domain_lookup_by_id_args*);
extern bool_t xdr_remote_domain_lookup_by_id_ret (XDR *,
remote_domain_lookup_by_id_ret*);
extern bool_t xdr_remote_domain_lookup_by_uuid_args (XDR *,
Re: [libvirt] [PATCH] fix index creation for disks {sd,hd,xvd,vd}z
On Wed, Oct 08, 2008 at 06:23:05PM -0700, Chris Wright wrote:
Calling virDiskNameToIndex with a disk name {sd,hd,xvd,vd}z, such as
vdaa, generates a bogus index. Account for iterations through the loop.
Old behaviour:
vda - 0
vdz - 25
vdaa - 0
vdaz - 25
New behaviour:
vda - 0
vdz - 25
vdaa - 26
vdaz - 51
This was discovered by Sanjay Rao, thanks for the report.
ACK.
Good to know someone's testing scalability with 26 disks :-)
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Support SDL configuration for QEMU driver
On Thu, Oct 09, 2008 at 04:31:49PM +0200, Jim Meyering wrote:
Daniel P. Berrange [EMAIL PROTECTED] wrote:
+#define ADD_ENV_COPY(envname) \
+do {\
+char *val = getenv(envname);\
+ADD_ENV_SPACE; \
+if (val != NULL \
+(qenv[qenvc++] = strdup(val)) == NULL) \
+goto no_memory; \
+} while (0)
Doesn't this need to be adding envname=val strings,
rather than just val?
If it works as-is, then maybe none of these is actually used
(or at least they're not exercised in tests).
No it doesn't work is exercised in the tests I forgot to run them :-)
I also needed to explicitly set the env vars to predictable values
in the test suite.
...
+tmp = qenv;
+len = 0;
+while (*tmp) {
+if (actualargv[0])
+strcat(actualargv, );
+strcat(actualargv, *tmp);
+tmp++;
+}
tmp = argv;
len = 0;
No big deal, but both of those len = 0 assignments
can be removed, since len is no longer used.
Yep, killed them off.
Daniel
diff --git a/docs/libvirt.rng b/docs/libvirt.rng
--- a/docs/libvirt.rng
+++ b/docs/libvirt.rng
@@ -645,9 +645,21 @@
define name='graphic'
element name='graphics'
choice
-attribute name='type'
- valuesdl/value
- /attribute
+ group
+ attribute name='type'
+ valuesdl/value
+ /attribute
+ optional
+ attribute name='display'
+ text/
+ /attribute
+ /optional
+ optional
+ attribute name='xauth'
+ text/
+ /attribute
+ /optional
+ /group
group
attribute name='type'
valuevnc/value
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -28,6 +28,7 @@
#include limits.h
#include sys/types.h
#include sys/stat.h
+#include stdlib.h
#include unistd.h
#include errno.h
#include fcntl.h
@@ -689,6 +690,7 @@ int qemudBuildCommandLine(virConnectPtr
virDomainObjPtr vm,
unsigned int qemuCmdFlags,
const char ***retargv,
+ const char ***retenv,
int **tapfds,
int *ntapfds,
const char *migrateFrom) {
@@ -707,6 +709,8 @@ int qemudBuildCommandLine(virConnectPtr
int disableKQEMU = 0;
int qargc = 0, qarga = 0;
const char **qargv = NULL;
+int qenvc = 0, qenva = 0;
+const char **qenv = NULL;
const char *emulator;
uname(ut);
@@ -754,14 +758,59 @@ int qemudBuildCommandLine(virConnectPtr
do {\
ADD_ARG_LIT(-usbdevice); \
ADD_ARG_SPACE; \
-if ((asprintf((char **)(qargv[qargc++]), disk:%s, thisarg)) == -1)
{\
+if ((asprintf((char **)(qargv[qargc++]), \
+ disk:%s, thisarg)) == -1) { \
qargv[qargc-1] = NULL; \
goto no_memory; \
} \
} while (0)
+#define ADD_ENV_SPACE \
+do {\
+if (qenvc == qenva) { \
+qenva += 10;\
+if (VIR_REALLOC_N(qenv, qenva) 0) \
+goto no_memory; \
+} \
+} while (0)
+
+#define ADD_ENV(thisarg)\
+do {\
+ADD_ENV_SPACE; \
+qenv[qenvc++] = thisarg;\
+} while (0)
+
+#define ADD_ENV_LIT(thisarg)\
+do {\
+ADD_ENV_SPACE; \
+if ((qenv[qenvc++] = strdup(thisarg)) == NULL) \
+goto no_memory; \
+} while (0)
+
+#define
