Re: [libvirt] [RFC PATCH] lxc: don't return error on GetInfo when cgroups not yet set up
On Mon, Oct 03, 2011 at 02:03:18PM -0500, Serge E. Hallyn wrote: Quoting Daniel P. Berrange (berra...@redhat.com): The LXC controller 'main' method received the handshake FD and invokes lxcControllerRun(). This method does various setup tasks, in particular the following: if (lxcSetContainerResources(def) 0) goto cleanup; ... ... if (lxcContainerSendContinue(handshakefd) 0) { virReportSystemError(errno, %s, _(error sending continue signal to parent)); goto cleanup; } VIR_FORCE_CLOSE(handshakefd); Thanks, Daniel. You're right! This is fixed in git, by the patch 'lxc: controller: Improve container error reporting' (which does much more than it says :). The following patch is how I had just fixed 0.9.2 this morning. It'll be nicer if I can get the git commit cherrypicked. I can't wait till I can upgrade! Ah, that explains it :-) Good to know its fixed then. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Document that ff callbacks need to be invoked from a clean stack
On Mon, Oct 03, 2011 at 10:41:17PM +0200, Guido Günther wrote: On Tue, Aug 16, 2011 at 02:24:53PM +0200, Guido Günther wrote: Hi Daniel, On Sat, Aug 13, 2011 at 08:57:45PM -0700, Daniel P. Berrange wrote: On Fri, Aug 12, 2011 at 11:54:28PM +0200, Guido Günther wrote: [..snip..] In the default libvirt event loop, the 'ff' callback is always invoked from a clean stack in the event loop, so you never have this problem with re-entrancy. Working around this by removing the locks from virNetSocketRemoveIOCallback leads to another deadlock: Yeah this is not a viable approach. Sure. This was only to see what else fails. I didn't see a simple way to fix this but would welcome any suggestions. IMHO we just have to document that event loop implementations should make sure that the 'ff' callbacks are always invoked from a clean stack. In the case of virt-viewer, this means changing it to register a g_idle callback function to invoke the 'ff' callback. Patch for virt-viewer attached. I'll come up with a doc patch for libvirt once I have a bit more time. As promised here's the patch for libvirt. O.k. to apply? Cheers, -- Guido From c2e2be4e377871ace12bb7adfde130e9b8779ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org Date: Mon, 3 Oct 2011 22:24:13 +0200 Subject: [PATCH] Document that ff callbacks need to be invoked from a clean stack. --- include/libvirt/libvirt.h.in |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a3c581d..bd7a0f7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2243,13 +2243,15 @@ typedef void (*virEventHandleCallback)(int watch, int fd, int events, void *opaq * @opaque: user data to pass to the callback * @ff: the callback invoked to free opaque data blob * - * Part of the EventImpl, this callback Adds a file handle callback to + * Part of the EventImpl, this callback adds a file handle callback to * listen for specific events. The same file handle can be registered * multiple times provided the requested event sets are non-overlapping * * If the opaque user data requires free'ing when the handle * is unregistered, then a 2nd callback can be supplied for - * this purpose. + * this purpose. This callback needs to be invoked from a clean stack. + * If 'ff' callbacks are invoked directly from the virEventRemoveHandleFunc + * they will likely deadlock in libvirt. * * Returns a handle watch number to be used for updating * and unregistering for events ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix migration with dname
Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. --- Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. src/qemu/qemu_domain.c|1 + src/qemu/qemu_domain.h|1 + src/qemu/qemu_migration.c | 19 --- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3ad192..65f721a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -231,6 +231,7 @@ static void qemuDomainObjPrivateFree(void *data) qemuDomainObjFreeJob(priv); VIR_FREE(priv-vcpupids); VIR_FREE(priv-lockState); +VIR_FREE(priv-origname); /* This should never be non-NULL if we get here, but just in case... */ if (priv-mon) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cdf1375..d9f323c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -126,6 +126,7 @@ struct _qemuDomainObjPrivate { int jobs_queued; unsigned long migMaxBandwidth; +char *origname; }; struct qemuDomainWatchdogEvent diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1122dab..4516231 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -254,12 +254,18 @@ error: static qemuMigrationCookiePtr qemuMigrationCookieNew(virDomainObjPtr dom) { +qemuDomainObjPrivatePtr priv = dom-privateData; qemuMigrationCookiePtr mig = NULL; +const char *name; if (VIR_ALLOC(mig) 0) goto no_memory; -if (!(mig-name = strdup(dom-def-name))) +if (priv-origname) +name = priv-origname; +else +name = dom-def-name; +if (!(mig-name = strdup(name))) goto no_memory; memcpy(mig-uuid, dom-def-uuid, VIR_UUID_BUFLEN); @@ -1064,6 +1070,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, unsigned long long now; qemuMigrationCookiePtr mig = NULL; bool tunnel = !!st; +char *origname = NULL; if (virTimeMs(now) 0) return -1; @@ -1078,7 +1085,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* Target domain name, maybe renamed. */ if (dname) { -VIR_FREE(def-name); +origname = def-name; def-name = strdup(dname); if (def-name == NULL) goto cleanup; @@ -1095,6 +1102,8 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, } def = NULL; priv = vm-privateData; +priv-origname = origname; +origname = NULL; if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, QEMU_MIGRATION_COOKIE_LOCKSTATE))) @@ -1175,6 +1184,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, ret = 0; cleanup: +VIR_FREE(origname); virDomainDefFree(def); VIR_FORCE_CLOSE(dataFD[0]); VIR_FORCE_CLOSE(dataFD[1]); @@ -2542,6 +2552,7 @@ qemuMigrationFinish(struct qemud_driver *driver, qemuMigrationCookiePtr mig = NULL; virErrorPtr orig_err = NULL; int cookie_flags = 0; +qemuDomainObjPrivatePtr priv = vm-privateData; VIR_DEBUG(driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d, @@ -2695,8 +2706,10 @@ endjob: } cleanup: -if (vm) +if (vm) { +VIR_FREE(priv-origname); virDomainObjUnlock(vm); +} if (event) qemuDomainEventQueue(driver, event); qemuMigrationCookieFree(mig); -- 1.7.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Adding new filesystem 'proxy' to 9p
That is the case if the proxy helper code is perfectly written. I am trying to think about the scenario where there is a bug (eg heap corruption / stack overflow) which allows a malicious non-root QEMU process to exploit the proxy helper to run code that it was *not* intended to run. If the proxy helper is running root with all capabilities, then a bug in the proxy helper can easily turn into a full root exploit. If the proxy helper starts as root, chroots, and then immediately drops to a non-root user, keeping only the CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_FOWNER and CAP_DAC_READ_SEARCH capabilities, then a bug in the proxy helper can only be used to access files within the designated 9pfs export. If the exported directory does not contain any important host system files, then it is unlikely it can be used to create a full root exploit. Thanks Daniel, I will add 'capabiliies' to proxy helper. CAP_FOWNER capability also need. I am working on the patches. I will post them in few days. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
On Mon, Oct 03, 2011 at 10:46:18PM -0600, Jim Fehlig wrote: Adam Litke wrote: Hi Jim. I was testing this and found that I could not boot from the sata disks I defined. When I switch them back to ide, they can be booted just fine. Perhaps something is missing from the boot order logic? Hmm, I didn't notice this in my testing. sda in the below config contained /boot. Can you provide the domain config? With the following config, I end up booting from the cdrom. If I take away the cdrom definition completely, I am unable to boot at all. domain type='kvm' nameblockPull-test/name memory262144/memory currentMemory262144/currentMemory vcpu1/vcpu os type arch='x86_64' machine='pc-0.13'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashrestart/on_crash devices emulator/home/aglitke/src/qemu/x86_64-softmmu/qemu-system-x86_64/emulator disk type='file' device='disk' driver name='qemu' type='qed'/ source file='/home/aglitke/vm/sata-test/disk1.qed' / target dev='sda' bus='sata'/ /disk disk type='file' device='disk' driver name='qemu' type='qed'/ source file='/home/aglitke/vm/sata-test/disk2.qed' / target dev='sdb' bus='sata'/ /disk disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/home/aglitke/vm/images/natty-alternate-amd64.iso' / target dev='hda' bus='ide'/ /disk interface type=network source network=default / /interface graphics type='vnc' port='-1' autoport='yes'/ /devices /domain -- Adam Litke a...@us.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: fill in bandwidth from portgroup for all forward modes
On Tue, Oct 04, 2011 at 12:17:27AM -0400, Laine Stump wrote: This patch is a fix for: https://bugzilla.redhat.com/show_bug.cgi?id=743176 which was discovered by Dan Berrange while making bandwidth configuration work for LXC guests. Background: Although virtportprofile data from a network portgroup is only applicable for direct mode interfaces, the code that copies bandwidth data from the portgroup was also only being executed in the case of direct mode interfaces. The result was that interfaces using traditional virtual networks (forward mode='nat|route|none'), and those using a host bridge for forwarding, would not pick up bandwidth data from a portgroup defined in the network. This patch moves that code outside the conditional, so that bandwidth information is *alway* copied from the appropriate portgroup (unless the interface definition itself already has bandwidth information, which would take precedence over what's in the portgroup anyway). ACK, this works in my test Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make LXC work with new network configuration types
From: Daniel P. Berrange berra...@redhat.com If using one of the new non-NAT/routed virtual network configurations, the LXC driver would not know how to setup the VETH devices. Adding in calls to setup the actual network configuration at VM startup and cleanup when shutting down fixes this. * src/lxc/lxc_driver.c: Setup/cleanup actual net devs --- src/lxc/lxc_driver.c | 18 -- 1 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c8e6119..c475887 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -54,6 +54,7 @@ #include fdstream.h #include domain_audit.h #include domain_nwfilter.h +#include network/bridge_driver.h #define VIR_FROM_THIS VIR_FROM_LXC @@ -1042,6 +1043,8 @@ static void lxcVmCleanup(lxc_driver_t *driver, for (i = 0 ; i vm-def-nnets ; i++) { vethInterfaceUpOrDown(vm-def-nets[i]-ifname, 0); vethDelete(vm-def-nets[i]-ifname); + +networkReleaseActualDevice(vm-def-nets[i]); } virDomainConfVMNWFilterTeardown(vm); @@ -1093,7 +1096,14 @@ static int lxcSetupInterfaces(virConnectPtr conn, char *parentVeth; char *containerVeth = NULL; -switch (def-nets[i]-type) { +/* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ +if (networkAllocateActualDevice(def-nets[i]) 0) +goto error_exit; + +switch (virDomainNetGetActualType(def-nets[i])) { case VIR_DOMAIN_NET_TYPE_NETWORK: { virNetworkPtr network; @@ -1110,7 +1120,7 @@ static int lxcSetupInterfaces(virConnectPtr conn, break; } case VIR_DOMAIN_NET_TYPE_BRIDGE: -bridge = def-nets[i]-data.bridge.brname; +bridge = virDomainNetGetActualBridgeName(def-nets[i]); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -1183,6 +1193,10 @@ static int lxcSetupInterfaces(virConnectPtr conn, error_exit: brShutdown(brctl); +if (rc != 0) { +for (i = 0 ; i def-nnets ; i++) +networkReleaseActualDevice(def-nets[i]); +} return rc; } -- 1.7.6.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 12:06:10PM +0200, Jiri Denemark wrote: Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. What is the actual error we get ? Is it that the 'Confirm' method raises an error Incoming cookie data had unexpected name ? --- Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? ACK, if my two questions here are correct Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: fill in bandwidth from portgroup for all forward modes
On 10/04/2011 08:49 AM, Daniel P. Berrange wrote: On Tue, Oct 04, 2011 at 12:17:27AM -0400, Laine Stump wrote: This patch is a fix for: https://bugzilla.redhat.com/show_bug.cgi?id=743176 which was discovered by Dan Berrange while making bandwidth configuration work for LXC guests. Background: Although virtportprofile data from a network portgroup is only applicable for direct mode interfaces, the code that copies bandwidth data from the portgroup was also only being executed in the case of direct mode interfaces. The result was that interfaces using traditional virtual networks (forward mode='nat|route|none'), and those using a host bridge for forwarding, would not pick up bandwidth data from a portgroup defined in the network. This patch moves that code outside the conditional, so that bandwidth information is *alway* copied from the appropriate portgroup (unless theinterface definition itself already has bandwidth information, which would take precedence over what's in the portgroup anyway). ACK, this works in my test Pushed. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 14:03:34 +0100, Daniel P. Berrange wrote: On Tue, Oct 04, 2011 at 12:06:10PM +0200, Jiri Denemark wrote: Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. What is the actual error we get ? Is it that the 'Confirm' method raises an error Incoming cookie data had unexpected name ? It's actually the Prepare method that fails but the error is the same (obviously since it comes from EatCookie). Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make LXC work with new network configuration types
On 10/04/2011 08:52 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com If using one of the new non-NAT/routed virtual network configurations, the LXC driver would not know how to setup the VETH devices. Adding in calls to setup the actual network configuration at VM startup and cleanup when shutting down fixes this. * src/lxc/lxc_driver.c: Setup/cleanup actual net devs ACK. This looks like proper usage of the functions in the proper place. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On Tue, Oct 04, 2011 at 15:16:28 +0200, Jiri Denemark wrote: On Tue, Oct 04, 2011 at 14:03:34 +0100, Daniel P. Berrange wrote: On Tue, Oct 04, 2011 at 12:06:10PM +0200, Jiri Denemark wrote: Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. What is the actual error we get ? Is it that the 'Confirm' method raises an error Incoming cookie data had unexpected name ? It's actually the Prepare method that fails but the error is the same (obviously since it comes from EatCookie). Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) So I pushed this, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume
On 09/29/2011 11:26 PM, MATSUDA, Daiki wrote: I tried the new snapshot function implemented by Eric Blake. It works very well for QCOW2 disk image system. But I often use LVM2 volume for QEMU virtual machines and tried to take disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only). So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2 volume and create QCOW2 snapshot image. In addition, domain's configuration file is replaced to use snapshot disk image instead of LVM2 volume. It sounds like virsh did what it was told, but that you told it so little information that it had to fill in the gaps and choose its own destination file name (hence the .1317357844 suffix after the action). Your situation sounds like a case where you may want a bit more control over the destination file name. virsh outputs virsh # snapshot-create LVM2_dom --disk-only Domain snapshot 1317357844 created And I confirmed that the qcow2 image file is created under /dev/VG1 # file /dev/VG1/LVM2_dom.1317357844 /dev/VG1/LVM2_dom.1317686816: Qemu Image, Format: Qcow , Version: 2 configuration file from disk type='block' device='disk driver name='qemu' type='raw' cache='none'/ source dev='dev/VG1/LVM2_dom'/ to disk type='block' device='disk driver name='qemu' type='qcow2' cache='none'/ source dev='dev/VG1/LVM2_dom.1317357844'/ Are you pasting literal chunks, or retyping this? You're missing the / in front of dev/VG1, so I can't help but wonder what else might have been mistyped. I am sorry. They are my mistyping and correct is '/dev/VG1/LVM2_dom' and '/dev/VG1/LVM2_dom.1317357844'. After then, the domain runs well till it is shutdowned. I'm surprised that it got that far - generally, libvirt can't create random regular files under the /dev/VG1/ device-mapper hierarchy, and if a file can't be created, then what was open() doing, and what did qemu actually do? It may be worth looking at lsof says that qemu has open, if you still have it running. Or it may be that you've found a bug in libvirt and/or qemu for not accurately reporting failure to create the snapshot image. But in reality the file is created by qemu-kvm with snapshot_blkdev in qemu-monitor command. I use libvirt-0.9.6 and qemu-kvm-0.12.12.1.2-2.160 and August's snapshot. I think we need to step back a bit and look at the bigger picture. Do you want your new qcow2 file to be its own LVM volume (in which case, I'd suggest that you pre-create the LVM volume, and pass in the file name within /dev/VG1/ of the existing block device to use), or are you okay with it being a regular file (in which case, I'd suggest that you do not pre-create the file, but that you still pass in the desired filename to some absolute path that lives outside of /dev/)? No, I do not want qcow2 file on LVM volume. I found the bug at simply tesing. I will never create the snapshot with 'snapshot-create ... --disk-only' for LVM2 volume, but users will try... So, I think that it is better not to refuse in libvirt. Either way, it sounds like you do _not_ want libvirt to be generating its own filename, since libvirt only knows how to generate a name in the same directory as the snapshot is taking place, but your lvm is in a special directory. To do this, you either need to create an XML file yourself, and call 'virsh snapshot-create dom --disk-only file', or you need to have virsh create the xml for you with 'virsh snapshot-create-as dom --disk-only vda,file=/path/to/file'. You can see the xml that snapshot-create-as would generate (in case you need to further fine-tune it for your own use in snapshot-create) via the --print-xml option. I started the domain, but it does not with following error virtsh # start LVM2_dom error: Failed to start domain LVM2_dom error: 内部エラー Process exited while reading console log output: char device redirected to /dev/pts/7 qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid argument. That makes sense, if that file doesn't exist (but why qemu didn't reject the snapshot in the first place still remains to be investigated). I think that if the volume but qcow2 is given libvirt should be refuse, No, qemu does just fine with a non-qcow2 backing file. The real problem here is that the new qcow2 file was not created, not the type of the original file. At least its phenomenon is reproduced easily. So I hope you test. e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver type. But currently the structures concerning with snapshot or disk has no member to hold such a volume driver information. In addition, as we want to add the LVM2 and other volume snapshot function, we hope you add its information and fix. Yes, I have much longer-term plans for refactoring device snapshots to move the work into more virStorageVolPtr operations, and teach virDomainSnapshotCreateXML to reuse virStorageVol actions rather
Re: [libvirt] [PATCH] qemu: Fix migration with dname
On 10/04/2011 07:48 AM, Jiri Denemark wrote: IIUC, you are trying to fix this, by making sure that the 'Finish' method encodes the original name in the cookie, not the new name ? Yes, although the complete picture is that incoming (from the POV of destination libvirtd) cookie is checked against the original name instead of the new one and cookies generated by destination libvirtd contain the original name. It applies to Prepare as well as Finish. ACK, if my two questions here are correct Mostly correct so I take it as ACK :-) Quick questions (from a latecomer to the thread): what happens if I use both the @dname and @dxml arguments? Are we properly requiring that the new name in both arguments match, and rejecting the migration as impossible otherwise (since you can't request two different names), or are we having one of the two names take priority over the other? Also, if @dname is NULL but @dxml is provided, I think that we currently refuse migration to a server that only understands v2 migration (since only v3 can take @dxml). Can @dxml in isolation be used to change the name, without the use of @dname? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix 'make distcheck' with pdwtags installed
I am getting this failure with 'make distcheck': GEN../../src/remote_protocol-structs /bin/sh: ../../src/remote_protocol-structs-t: Permission denied make[4]: *** [../../src/remote_protocol-structs] Error 1 since it attempts a sub-run of a VPATH 'make check' where $(srcdir) is intentionally read-only. I'm not sure which commit introduced the problem, although I suspect it was around 62dee6f when I refactored protocol struct checking to be more powerful. $(@F) is required by POSIX, and although it is not yet portable to all make implementations, we already require GNU make. * src/Makefile.am (PDWTAGS): Generate temp file into current directory, since $(srcdir) is read-only during distcheck. --- src/Makefile.am |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 738ee91..9650139 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -255,9 +255,9 @@ PDWTAGS = \ -e 'exit 8;'\ -e ' }'\ -e '}' \ -$@-t; \ +$(@F)-t; \ case $$? in 8) exit 0;; 0) ;; *) exit 1;; esac; \ - diff -u $@-t $@; st=$$?; rm -f $@-t; exit $$st; \ + diff -u $(@F)-t $@; st=$$?; rm -f $(@F)-t; exit $$st; \ else\ echo 'WARNING: you lack pdwtags; skipping the $@ test' 2; \ echo 'WARNING: install the dwarves package to get pdwtags' 2; \ -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] init: raise default system aio limits
https://bugzilla.redhat.com/show_bug.cgi?id=740899 documents that if qemu uses aio=native for its disks, then it consumes 128 aio requests per disk. On a host with multiple guests, this can quickly run out of kernel aio requests with the default aio-max-nr of 65536. Kernel developers have confirmed that there is no up-front cost to raising this limit (a larger limit merely implies that more aio requests can be issued in parallel, which in turn will result in more kernel memory allocation if the system really does use that many requests). Since the system default limit prevents 256 disks, which is well within libvirt's current scalability, this patch installs a file to raise the limit and document it in case a system administrator has further cause to tune the limit. The install only works on platforms new enough to source /etc/sysctl.d/* alongside /etc/sysctl.conf (F14 and RHEL 6). * daemon/libvirtd.sysctl: New file. * daemon/Makefile.am (EXTRA_DIST): Ship it. (install-init, uninstall-init): Install it. * libvirt.spec.in (%files): Include it in rpm. --- So far, I've confirmed that this passes 'make distcheck' (well, once https://www.redhat.com/archives/libvir-list/2011-October/msg00074.html is applied) and 'make rpm', although I didn't actually try installing the resulting rpm nor rebooting a system to see if it takes effect. daemon/Makefile.am | 11 --- daemon/libvirtd.sysctl |8 libvirt.spec.in|5 + 3 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 daemon/libvirtd.sysctl diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 690bf85..1cf2b73 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -38,6 +38,7 @@ EXTRA_DIST = \ libvirtd.policy-1 \ libvirtd.sasl \ libvirtd.sysconf\ + libvirtd.sysctl \ libvirtd.aug\ libvirtd.logrotate.in \ libvirtd.qemu.logrotate.in \ @@ -252,16 +253,20 @@ install-logrotate: $(LOGROTATE_CONFS) if LIBVIRT_INIT_SCRIPT_RED_HAT install-init: libvirtd.init - mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d + mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d \ + $(DESTDIR)$(sysconfdir)/sysconfig \ + $(DESTDIR)$(sysconfdir)/sysctl.d $(INSTALL_SCRIPT) libvirtd.init \ $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirtd - mkdir -p $(DESTDIR)$(sysconfdir)/sysconfig $(INSTALL_DATA) $(srcdir)/libvirtd.sysconf \ $(DESTDIR)$(sysconfdir)/sysconfig/libvirtd + $(INSTALL_DATA) $(srcdir)/libvirtd.sysctl \ + $(DESTDIR)$(sysconfdir)/sysctl.d/libvirtd uninstall-init: rm -f $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirtd \ - $(DESTDIR)$(sysconfdir)/sysconfig/libvirtd + $(DESTDIR)$(sysconfdir)/sysconfig/libvirtd \ + $(DESTDIR)$(sysconfdir)/sysctl.d/libvirtd BUILT_SOURCES += libvirtd.init diff --git a/daemon/libvirtd.sysctl b/daemon/libvirtd.sysctl new file mode 100644 index 000..3c70884 --- /dev/null +++ b/daemon/libvirtd.sysctl @@ -0,0 +1,8 @@ +# The kernel allocates aio memory on demand, and this number limits the +# number of parallel aio requests; the only drawback of a larger limit is +# that a malicious guest could issue parallel requests to cause the kernel +# to set aside memory. Set this number at least as large as +# 128 * (number of virtual disks on the host) +# Libvirt uses a default of 1M requests to allow 8k disks, with at most +# 64M of kernel memory if all disks hit an aio request at the same time. +fs.aio-max-size = 1048576 diff --git a/libvirt.spec.in b/libvirt.spec.in index b87e3f6..9f5797a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -962,6 +962,11 @@ fi %doc daemon/libvirtd.upstart %config(noreplace) %{_sysconfdir}/sysconfig/libvirtd %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf +%if 0%{?fedora} = 14 || 0%{?rhel} = 6 +%config(noreplace) %{_sysconfdir}/sysctl.d/libvirtd +%else +rm -f $RPM_BUILD_ROOT%{_sysconfdir}/sysctl.d/libvirtd +%endif %if %{with_dtrace} %{_datadir}/systemtap/tapset/libvirtd.stp %endif -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] security_dac: don't chown iso file
Quoting Serge E. Hallyn (serge.hal...@canonical.com): isos are read-only, so libvirt doesn't need to chown them. In one of our testing setups, libvirt uses mirrorred isos. Since libvirt chowns the files, (and especially does not chown them back) the mirror refuses to update the iso. This patch prevents libvirt from chowning files. Does this seem reasonable? Hi, any feedback on this? Does it seem ok? thanks, -serge Signed-off-by: Serge Hallyn serge.hal...@canonical.com --- src/security/security_dac.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index af02236..e7db324 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -555,6 +555,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, /* XXX fixme - we need to recursively label the entire tree :-( */ if (vm-def-disks[i]-type == VIR_DOMAIN_DISK_TYPE_DIR) continue; + if (vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_CDROM) + continue; if (virSecurityDACSetSecurityImageLabel(mgr, vm, vm-def-disks[i]) 0) -- 1.7.5.4 -- 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 0/2] qemu: fix disk error_policy problems
These two patches are related to: https://bugzilla.redhat.com/show_bug.cgi?id=730909 The first removes the obvious problem of attempting to feed qemu invalid options. The second makes it possible to specify a different policy for read errors and write errors. 1/2 is written such that 2/2 will apply with minimal rewriting of existing code (that's why the qemu_command.c part may seem a bit obtuse at first). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: add separate rerror_policy for disk errors
libvirt's XML only had a single attribute, error_policy to control both read and write error policy, but qemu has separate settings for these. In one case (enospc) a policy is allowed for write errors but not read errors. This patch adds a separate attribute that sets only the read error policy. If just error_policy is set, it will apply to both read and write error policy (previous behavior), but if the new rerror_policy attribute is set, it will override error_policy for read errors only. --- docs/formatdomain.html.in | 16 +--- src/conf/domain_conf.c| 16 src/conf/domain_conf.h|3 ++- src/qemu/qemu_command.c |5 - 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 593adcb..5265b21 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1008,9 +1008,19 @@ /li li The optional codeerror_policy/code attribute controls -how the hypervisor will behave on an error, possible -values are stop, ignore, and enospace. -span class=sinceSince 0.8.0/span +how the hypervisor will behave on a disk read or write +error, possible values are stop, ignore, and +enospace.span class=sinceSince 0.8.0/span There is +also an optional codererror_policy/code that controls +behavior for read errors only.span class=sinceSince +0.9.7/space. If no rerror_policy is given, error_policy +is used for both read and write errors. If rerror_policy +is given, it overrides the codeerror_policy/code for +read errors. Also note that enospace is not a valid +policy for read errors, so if codeerror_policy/code is +set to enospace and no codererror_policy/code is +given, the read error policy will be automatically set to +ignore. /li li The optional codeio/code attribute controls specific diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9007ce..95e1a9a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2297,6 +2297,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *bus = NULL; char *cachetag = NULL; char *error_policy = NULL; +char *rerror_policy = NULL; char *iotag = NULL; char *ioeventfd = NULL; char *event_idx = NULL; @@ -2416,6 +2417,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, driverType = virXMLPropString(cur, type); cachetag = virXMLPropString(cur, cache); error_policy = virXMLPropString(cur, error_policy); +rerror_policy = virXMLPropString(cur, rerror_policy); iotag = virXMLPropString(cur, io); ioeventfd = virXMLPropString(cur, ioeventfd); event_idx = virXMLPropString(cur, event_idx); @@ -2560,6 +2562,16 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } +if (rerror_policy +(((def-rerror_policy + = virDomainDiskErrorPolicyTypeFromString(error_policy)) 0) || + (def-rerror_policy == VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE))) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _(unknown disk read error policy '%s'), + rerror_policy); +goto error; +} + if (iotag) { if ((def-iomode = virDomainDiskIoTypeFromString(iotag)) 0 || def-iomode == VIR_DOMAIN_DISK_IO_DEFAULT) { @@ -2667,6 +2679,7 @@ cleanup: VIR_FREE(driverName); VIR_FREE(cachetag); VIR_FREE(error_policy); +VIR_FREE(rerror_policy); VIR_FREE(iotag); VIR_FREE(ioeventfd); VIR_FREE(event_idx); @@ -9127,6 +9140,7 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *bus = virDomainDiskBusTypeToString(def-bus); const char *cachemode = virDomainDiskCacheTypeToString(def-cachemode); const char *error_policy = virDomainDiskErrorPolicyTypeToString(def-error_policy); +const char *rerror_policy = virDomainDiskErrorPolicyTypeToString(def-rerror_policy); const char *iomode = virDomainDiskIoTypeToString(def-iomode); const char *ioeventfd = virDomainIoEventFdTypeToString(def-ioeventfd); const char *event_idx = virDomainVirtioEventIdxTypeToString(def-event_idx); @@ -9177,6 +9191,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, cache='%s', cachemode); if (def-error_policy) virBufferAsprintf(buf, error_policy='%s', error_policy); +if (def-rerror_policy) +virBufferAsprintf(buf, rerror_policy='%s', rerror_policy); if (def-iomode) virBufferAsprintf(buf, io='%s', iomode); if (def-ioeventfd) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bc41d34..35eacc7 100644 ---
[libvirt] [PATCH 1/2] qemu: correct misspelled 'enospc' option, and only use for werror
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=730909 When support for setting the qemu disk error policy to 'enospc' was added, it was inadvertantly as enospace. This patch corrects that on the qemu commandline (while retaining the enospace spelling for libvirt's XML. Also, while examining the qemu source, I found that enospc is not allowed for the read error policy, only for write error policy (makes sense). Since libvirt currently only has a single error policy setting, when enospace is selected, the read error policy is set to ignore. --- src/qemu/qemu_command.c | 22 ++ 1 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ff83e2d..123bcab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1692,11 +1692,25 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } if (qemuCapsGet(qemuCaps, QEMU_CAPS_MONITOR_JSON)) { -if (disk-error_policy) { -virBufferAsprintf(opt, ,werror=%s,rerror=%s, - virDomainDiskErrorPolicyTypeToString(disk-error_policy), - virDomainDiskErrorPolicyTypeToString(disk-error_policy)); +const char *wpolicy = NULL, *rpolicy = NULL; + +if (disk-error_policy) +wpolicy = virDomainDiskErrorPolicyTypeToString(disk-error_policy); +if (!rpolicy) +rpolicy = wpolicy; + +if (disk-error_policy == VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE) { +/* in the case of enospace, the option is spelled differently in qemu, + * and it's only valid for werror, not for rerror. + */ +wpolicy=enospc; +rpolicy=ignore; } + +if (wpolicy) +virBufferAsprintf(opt, ,werror=%s, wpolicy); +if (rpolicy) +virBufferAsprintf(opt, ,rerror=%s, rpolicy); } if (disk-iomode) { -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] init: raise default system aio limits
On 10/04/2011 12:59 PM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=740899 documents that if qemu uses aio=native for its disks, then it consumes 128 aio requests per disk. On a host with multiple guests, this can quickly run out of kernel aio requests with the default aio-max-nr of 65536. Kernel developers have confirmed that there is no up-front cost to raising this limit (a larger limit merely implies that more aio requests can be issued in parallel, which in turn will result in more kernel memory allocation if the system really does use that many requests). Since the system default limit prevents 256 disks, which is well within libvirt's current scalability, this patch installs a file to raise the limit and document it in case a system administrator has further cause to tune the limit. The install only works on platforms new enough to source /etc/sysctl.d/* alongside /etc/sysctl.conf (F14 and RHEL 6). ACK. This all makes sense and seems fairly thoroughly researched. (Would be good to actually try it, though) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Document that ff callbacks need to be invoked from a clean stack
On Tue, Oct 04, 2011 at 09:00:17AM +0100, Daniel P. Berrange wrote: On Mon, Oct 03, 2011 at 10:41:17PM +0200, Guido Günther wrote: On Tue, Aug 16, 2011 at 02:24:53PM +0200, Guido Günther wrote: Hi Daniel, On Sat, Aug 13, 2011 at 08:57:45PM -0700, Daniel P. Berrange wrote: On Fri, Aug 12, 2011 at 11:54:28PM +0200, Guido Günther wrote: [..snip..] In the default libvirt event loop, the 'ff' callback is always invoked from a clean stack in the event loop, so you never have this problem with re-entrancy. Working around this by removing the locks from virNetSocketRemoveIOCallback leads to another deadlock: Yeah this is not a viable approach. Sure. This was only to see what else fails. I didn't see a simple way to fix this but would welcome any suggestions. IMHO we just have to document that event loop implementations should make sure that the 'ff' callbacks are always invoked from a clean stack. In the case of virt-viewer, this means changing it to register a g_idle callback function to invoke the 'ff' callback. Patch for virt-viewer attached. I'll come up with a doc patch for libvirt once I have a bit more time. As promised here's the patch for libvirt. O.k. to apply? Cheers, -- Guido From c2e2be4e377871ace12bb7adfde130e9b8779ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org Date: Mon, 3 Oct 2011 22:24:13 +0200 Subject: [PATCH] Document that ff callbacks need to be invoked from a clean stack. --- include/libvirt/libvirt.h.in |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a3c581d..bd7a0f7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2243,13 +2243,15 @@ typedef void (*virEventHandleCallback)(int watch, int fd, int events, void *opaq * @opaque: user data to pass to the callback * @ff: the callback invoked to free opaque data blob * - * Part of the EventImpl, this callback Adds a file handle callback to + * Part of the EventImpl, this callback adds a file handle callback to * listen for specific events. The same file handle can be registered * multiple times provided the requested event sets are non-overlapping * * If the opaque user data requires free'ing when the handle * is unregistered, then a 2nd callback can be supplied for - * this purpose. + * this purpose. This callback needs to be invoked from a clean stack. + * If 'ff' callbacks are invoked directly from the virEventRemoveHandleFunc + * they will likely deadlock in libvirt. * * Returns a handle watch number to be used for updating * and unregistering for events ACK Pushed. Thanks. -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] snapshot: better virsh handling of missing current, parent
On 09/30/2011 12:19 PM, Eric Blake wrote: Previously, virsh 'snapshot-parent' and 'snapshot-current' were completely silent in the case where the code conclusively proved there was no parent or current snapshot, but differed in exit status; this silence caused some confusion on whether the commands worked. Furthermore, commit d1be48f introduced a regression where snapshot-parent would leak output about an unknown function, but only on the first attempt, when talking to an older server that lacks virDomainSnapshotGetParent. This changes things to consistenly report an error message and exit with status 1 when no snapshot exists, and to avoid leaking unknown function warnings when using fallbacks. * tools/virsh.c (vshGetSnapshotParent): Alter signature, to distinguish between real error and missing parent. Don't pollute last_error on success. (cmdSnapshotParent): Adjust caller. Always output message on failure. (vshGetSnapshotParent): Adjust caller. (cmdSnapshotCurrent): Always output message on failure. --- This patch is an RFC because of backwards-compatibility concerns: Currently, snapshot-current outputs nothing but has status 0 if there is no current snapshot; but snapshot-parent outputs nothing but has status 1 if there is no parent snapshot. Either way, we have an inconsistency that needs to be fixed, and gaining consistency means breaking backwards compatibility with at least one command. Approach 1 (this patch): change snapshot-parent and snapshot-current to always output something, whether to stdout on success or to stderr on failure, with lack of a snapshot being considered a failure (where snapshot-current used to treat it as success). Approach 2 (not written) since snapshot-current existed much longer, makesnapshot-parent consistent by giving status 0 when it is silent. Preferences? (I guess mine is approach 1, by evidence of this patch). The code all looks fine, so ACK to that. Not being very familiar with the snapshot stuff, I can't say that I have an valid opinion on whether it's better to log an error or exit silently when there is no parent. I guess I would defer to your opinion, since you're the person who's spent the most time dealing with snapshots lately :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: correct misspelled 'enospc' option, and only use for werror
On 10/04/2011 12:35 PM, Laine Stump wrote: This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=730909 When support for setting the qemu disk error policy to 'enospc' was added, it was inadvertantly as enospace. This patch corrects that on spelling and grammar s/inadvertantly/inadvertently spelled/ the qemu commandline (while retaining the enospace spelling for libvirt's XML. Also, while examining the qemu source, I found that enospc is not allowed for the read error policy, only for write error policy (makes sense). Since libvirt currently only has a single error policy setting, when enospace is selected, the read error policy is set to ignore. --- src/qemu/qemu_command.c | 22 ++ 1 files changed, 18 insertions(+), 4 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: add REVERT_FORCE to API
On 09/30/2011 02:52 PM, Eric Blake wrote: Although reverting to a snapshot is a form of data loss, this is normally expected. However, there are two cases where additional surprises (failure to run the reverted state, or a break in connectivity to the domain) can come into play. Requiring extra acknowledgment in these cases will make it less likely that someone can get into an unrecoverable state due to a default revert. Also create a new error code, so users can distinguish when forcing would make a difference, rather than having to blindly request force. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_REVERT_FORCE): New flag. * src/libvirt.c (virDomainRevertToSnapshot): Document it. * include/libvirt/virterror.h (VIR_ERR_SNAPSHOT_REVERT_RISKY): New error value. * src/util/virterror.c (virErrorMsg): Implement it. * tools/virsh.c (cmdDomainSnapshotRevert): Add --force to virsh. * tools/virsh.pod (snapshot-revert): Document it. --- include/libvirt/libvirt.h.in |1 + include/libvirt/virterror.h |2 ++ src/libvirt.c| 22 ++ src/util/virterror.c |6 ++ tools/virsh.c| 19 ++- tools/virsh.pod | 17 + 6 files changed, 66 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3c7f278..7403a9a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2736,6 +2736,7 @@ virDomainSnapshotPtr virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, typedef enum { VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 0, /* Run after revert */ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED = 1 1, /* Pause after revert */ +VIR_DOMAIN_SNAPSHOT_REVERT_FORCE = 1 2, /* Allow risky reverts */ } virDomainSnapshotRevertFlags; /* Revert the domain to a point-in-time snapshot. The diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 0aae622..0c98014 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -237,6 +237,8 @@ typedef enum { the given driver */ VIR_ERR_STORAGE_PROBE_FAILED = 75, /* storage pool proble failed */ VIR_ERR_STORAGE_POOL_BUILT = 76,/* storage pool already built */ +VIR_ERR_SNAPSHOT_REVERT_RISKY = 77, /* force was not requested for a + risky domain snapshot revert */ } virErrorNumber; /** diff --git a/src/libvirt.c b/src/libvirt.c index 2b2f6be..c2aabf7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16373,6 +16373,28 @@ error: * into an inactive state, so transient domains require the use of one * of these two flags. * + * Reverting to any snapshot discards all configuration changes made since + * the last snapshot. Additionally, reverting to a snapshot from a running + * domain is a form of data loss, since it discards whatever is in the + * guest's RAM at the time. However, the very nature of keeping snapshots + * implies the intent to roll back state, so no additional confirmation is + * normally required for these effects. + * + * However, there are two particular situations where reverting will + * be refused by default, and where @flags must include + * VIR_DOMAIN_SNAPSHOT_REVERT_FORCE to acknowledge the risks. 1) Any + * attempt to revert to a snapshot that lacks the metadata to perform + * ABI compatibility checks (generally the case for snapshots that + * lack a fulldomain when listed by virDomainSnapshotGetXMLDesc(), + * such as those created prior to libvirt 0.9.5). 2) Any attempt to + * revert a running domain to an active state that requires starting a + * new hypervisor instance rather than reusing the existing hypervisor + * (since this would terminate all connections to the domain, such as + * such as VNC or Spice graphics) - this condition arises from active + * snapshots that are provably ABI incomaptible, as well as from + * inactive snapshots with a request to start the domain after the + * revert. + * * Returns 0 if the creation is successful, -1 on error. */ int diff --git a/src/util/virterror.c b/src/util/virterror.c index 26c4981..5006fa2 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1204,6 +1204,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _(argument unsupported: %s); break; +case VIR_ERR_SNAPSHOT_REVERT_RISKY: +if (info == NULL) +errmsg = _(revert requires force); +else +errmsg = _(revert requires force: %s); +break; } return (errmsg); } diff --git a/tools/virsh.c b/tools/virsh.c index 655378c..4e79325 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13571,6 +13571,7 @@ static const vshCmdOptDef opts_snapshot_revert[] = { {snapshotname, VSH_OT_DATA, VSH_OFLAG_REQ, N_(snapshot name)},
Re: [libvirt] [PATCH 2/2] snapshot: enforce REVERT_FORCE on qemu
On 09/30/2011 02:52 PM, Eric Blake wrote: Implements the documentation for snapshot revert vs. force. Part of the patch tightens existing behavior (previously, reverting to an old snapshot withoutdomain was blindly attempted, now it requires force), while part of it relaxes behavior (previously, it was not possible to revert an active domain to an ABI-incompatible active snapshot, now force allows this transition). * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Check for risky situations, and allow force to get past them. --- src/qemu/qemu_driver.c | 47 +-- 1 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5110102..efd60a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9753,7 +9753,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * 7. paused - inactive: EVENT_STOPPED * 8. paused - running: EVENT_RESUMED * 9. paused - paused: none - * Also, several transitions occur even if we fail partway through. + * Also, several transitions occur even if we fail partway through, + * and use of FORCE can cause multiple transitions. */ qemuDriverLock(driver); @@ -9789,6 +9790,24 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, yet)); goto cleanup; } +if (!(flags VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { +if (!snap-def-dom) { +qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, +_(snapshot lacks domain '%s' rollback details), +snap-def-name); +goto cleanup; +} +if (virDomainObjIsActive(vm) +!(snap-def-state == VIR_DOMAIN_RUNNING + || snap-def-state == VIR_DOMAIN_PAUSED) +(flags (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { +qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, +_(must respawn qemu to start inactive snapshot)); +goto cleanup; +} +} + if (vm-current_snapshot) { vm-current_snapshot-def-current = false; @@ -9818,11 +9837,6 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_FREE(xml); if (!config) goto cleanup; -} else { -/* XXX Fail if VIR_DOMAIN_REVERT_FORCE is not set, rather than - * blindly hoping for the best. */ -VIR_WARN(snapshot is lacking rollback information for domain '%s', - snap-def-name); } if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) @@ -9843,10 +9857,22 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Transitions 5, 6, 8, 9 */ /* Check for ABI compatibility. */ if (config !virDomainDefCheckABIStability(vm-def, config)) { -/* XXX Add VIR_DOMAIN_REVERT_FORCE to permit killing - * and restarting a new qemu, since loadvm monitor - * command won't work. */ -goto endjob; +if (!(flags VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { +/* Alter existing error to give correct category. */ +virErrorPtr err = virGetLastError(); +err-code = VIR_ERR_SNAPSHOT_REVERT_RISKY; +goto endjob; +} +qemuProcessStop(driver, vm, 0, +VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); +virDomainAuditStop(vm, from-snapshot); +detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT; +event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + detail); +if (event) +qemuDomainEventQueue(driver, event); +goto load; } priv = vm-privateData; @@ -9882,6 +9908,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainObjAssignDef(vm, config, false); } else { /* Transitions 2, 3 */ +load: was_stopped = true; if (config) virDomainObjAssignDef(vm, config, false); ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: add REVERT_FORCE to API
On 10/04/2011 01:24 PM, Laine Stump wrote: - if (virDomainRevertToSnapshot(snapshot, flags) 0) + result = virDomainRevertToSnapshot(snapshot, flags); + if (result 0 force + last_error-code == VIR_ERR_SNAPSHOT_REVERT_RISKY) { + flags |= VIR_DOMAIN_SNAPSHOT_REVERT_FORCE; Are you not adding the FORCE flag the first time to allow for better interoperability with older libvirtd? Exactly. For example, libvirt 0.9.6 does not understand FORCE (and using it would error out with unrecognized flag bit), but just happens to revert to a snapshot created by 0.9.4 on the first call with flags == 0. Libvirt 0.9.7, in the same situation, will error out with the new error value when flags == 0, but with the new error, so that you know you can safely add FORCE. One other consideration is that 0.9.6 understands the new flag VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING, which 0.9.4 did not understand, so blindly looking for the 'unknown flag bit' error is not unique enough to be the indication that we could remove the --force bit and retry, because that still won't resolve on an 0.9.4 server. My choice of --force handling thus seems to be the one most likely to work for 0.9.4, 0.9.6, and 0.9.7, so that the user can blindly type 'virsh snapshot-revert dom old-snapshot --force' and attempt the revert regardless of server version. Conversely, I still wanted 'virsh snapshot-revert' _without_ --force will work in as many situations as the server supports, and that if a server rejects the operation because force is required, the error message will be specific enough to mention that. This is a good thing, so that users don't have to blindly add '--force' and expose themselves to unnecessary risk; rather, they should only add --force after an explicit message says what force would solve, and deciding that the risk of forcing for that particular case is worth it. That's the only question I had. It all looks fine - ACK. I'll wait for the 2/2 ack before applying this, then. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: add separate rerror_policy for disk errors
On 10/04/2011 12:35 PM, Laine Stump wrote: libvirt's XML only had a single attribute, error_policy to control both read and write error policy, but qemu has separate settings for these. In one case (enospc) a policy is allowed for write errors but not read errors. This patch adds a separate attribute that sets only the read error policy. If just error_policy is set, it will apply to both read and write error policy (previous behavior), but if the new rerror_policy attribute is set, it will override error_policy for read errors only. --- docs/formatdomain.html.in | 16 +--- Missing the patch to docs/schemas/domaincommon.rng Also, I'd like to see at least one addition to tests/qemuxml2argvdata that exposes the new command line. -span class=sinceSince 0.8.0/span +how the hypervisor will behave on a disk read or write +error, possible values are stop, ignore, and +enospace.span class=sinceSince 0.8.0/span There is +also an optionalcodererror_policy/code that controls +behavior for read errors only.span class=sinceSince +0.9.7/space. If no rerror_policy is given, error_policy +is used for both read and write errors. If rerror_policy +is given, it overrides thecodeerror_policy/code for +read errors. Also note that enospace is not a valid +policy for read errors, so ifcodeerror_policy/code is +set to enospace and nocodererror_policy/code is +given, the read error policy will be automatically set to +ignore. I think we also need to document that default can be used for either (or both) [r]error_policy as a way to rely on the hypervisor defaults. More on this below. @@ -2560,6 +2562,16 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } +if (rerror_policy +(((def-rerror_policy + = virDomainDiskErrorPolicyTypeFromString(error_policy)) 0) || virDomainDiskErrorPolicyTypeFromString converts default to 0, which means this is an undocumented value that we parse. In many existing cases, we use vir...TypeFromString() = 0 to reject parsing default. But here, you copied-and-pasted from error_policy, which also was using 0 (and silently accepting default). Now, as to _why_ it makes sense to allow default, consider the case where I want my qemu command line to have one, but not both, policies. How do I specify that in the XML? By doing: rerror_policy='stop' I get my desired: rerror=stop But what about the converse, for just werror on the command line? Using error_policy='stop' produces rerror=stop,werror=stop which isn't quite what I wanted. So I need: rerror_policy='default' error_policy='stop' to get exactly: werror=stop on the command line, which argues that default should be part of the documented XML for rerror_policy. And symmetry argues that we might as well then document default for error_policy. Hence, I think the following rng change covers things: diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index cffaac2..fffee9a 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -853,13 +853,25 @@ /attribute /define define name=driverErrorPolicy -attribute name=error_policy - choice -valuestop/value -valueignore/value -valueenospace/value - /choice -/attribute +optional + attribute name=error_policy +choice + valuedefault/value + valuestop/value + valueignore/value + valueenospace/value +/choice + /attribute +/optional +optional + attribute name=rerror_policy +choice + valuedefault/value + valuestop/value + valueignore/value +/choice + /attribute +/optional /define define name=driverIO attribute name=io @@ -9177,6 +9191,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, cache='%s', cachemode); if (def-error_policy) virBufferAsprintf(buf, error_policy='%s', error_policy); +if (def-rerror_policy) +virBufferAsprintf(buf, rerror_policy='%s', rerror_policy); Hmm, given my earlier arguments that documenting default makes sense, we would have to change the logic here to be that we always print both attributes, even if one of them is default, if at least one of them is something besides default: if (def-error_policy || def-rerror_policy) { virBufferAsprintf(buf, error_policy='%s' rerror_policy='%s', error_policy, rerror_policy } +++ b/src/qemu/qemu_command.c @@ -1696,6 +1696,8 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (disk-error_policy) wpolicy = virDomainDiskErrorPolicyTypeToString(disk-error_policy); +if (disk-rerror_policy) +rpolicy =
Re: [libvirt] [RFC PATCH] snapshot: better virsh handling of missing current, parent
On 10/04/2011 12:54 PM, Laine Stump wrote: On 09/30/2011 12:19 PM, Eric Blake wrote: Previously, virsh 'snapshot-parent' and 'snapshot-current' were completely silent in the case where the code conclusively proved there was no parent or current snapshot, but differed in exit status; this silence caused some confusion on whether the commands worked. Furthermore, commit d1be48f introduced a regression where snapshot-parent would leak output about an unknown function, but only on the first attempt, when talking to an older server that lacks virDomainSnapshotGetParent. This changes things to consistenly report an error message and exit with status 1 when no snapshot exists, and to avoid leaking unknown function warnings when using fallbacks. Preferences? (I guess mine is approach 1, by evidence of this patch). The code all looks fine, so ACK to that. Not being very familiar with the snapshot stuff, I can't say that I have an valid opinion on whether it's better to log an error or exit silently when there is no parent. I guess I would defer to your opinion, since you're the person who's spent the most time dealing with snapshots lately :-) I went ahead and pushed this as-is, on the basis that a common usage pattern, virsh snapshot-revert dom $(virsh snapshot-current --name), will do the right thing when there is no stdout, whether or not stderr was supplied, but having stderr available can help understand the situation when using the command in isolation. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] snapshot: enforce REVERT_FORCE on qemu
On 10/04/2011 01:26 PM, Laine Stump wrote: On 09/30/2011 02:52 PM, Eric Blake wrote: Implements the documentation for snapshot revert vs. force. Part of the patch tightens existing behavior (previously, reverting to an old snapshot withoutdomain was blindly attempted, now it requires force), while part of it relaxes behavior (previously, it was not possible to revert an active domain to an ABI-incompatible active snapshot, now force allows this transition). * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Check for risky situations, and allow force to get past them. ACK. Before pushing this, I'm running some sanity tests. So far, this test sequence (adjusted to the fixed code) shows where force helps with older snapshots (I'll send separate email for showing how force helps active ABI-incompatible snapshots): Test 1: $ virsh snapshot-create-as dom snap # offline domain with one qcow2 disk $ virsh edit dom # add a second qcow2 disk $ virsh snapshot-revert dom snap # offline revert doesn't need force $ virsh dumpxml dom # sure enough, second disk is gone $ virsh edit dom # add a second qcow2 disk $ virsh snapshot-dumpxml dom snap snap.xml $ virsh snapshot-delete --metadata dom snap $ sed -i '\|domain |,\|/domain| d' snap.xml $ virsh snapshot-create --redefine fedora_15-32 snap.xml # the delete --metadata/--redefine is necessary, so that libvirt # won't reuse domain from the prior definition $ virsh snapshot-revert dom snap # simulates reverting to 0.9.4 snapshot error: revert requires force: snapshot 'snap' lacks domain 'dom' rollback info $ virsh snapshot-revert dom snap --force error: internal error Child process (/usr/bin/qemu-img snapshot -a snap /path/to/second.qcow2) status unexpected: exit status 1 # See why we shouldn't have allowed blind revert? :) $ virsh edit dom # remove that second disk $ virsh snapshot-revert dom snap --force # now that we match expected state, the revert works as desired And here's what I have to squash in for test 1 to succeed as planned. diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 1a171cf..07c4fd4 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -9649,7 +9649,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainDefPtr config = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | - VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED, -1); + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED | + VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1); /* We have the following transitions, which create the following events: * 1. inactive - inactive: none @@ -9701,8 +9702,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (!(flags VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { if (!snap-def-dom) { qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, -_(snapshot lacks domain '%s' rollback details), -snap-def-name); +_(snapshot '%s' lacks domain '%s' rollback info), +snap-def-name, vm-def-name); goto cleanup; } if (virDomainObjIsActive(vm) diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c index 65f721a..dda53f3 100644 --- i/src/qemu/qemu_domain.c +++ w/src/qemu/qemu_domain.c @@ -1441,7 +1441,7 @@ qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, if (virRun(qemuimgarg, NULL) 0) { if (try_all) { VIR_WARN(skipping snapshot action on %s, - vm-def-disks[i]-info.alias); + vm-def-disks[i]-dst); skipped = true; continue; } -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] snapshot: enforce REVERT_FORCE on qemu
On 10/04/2011 04:02 PM, Eric Blake wrote: Before pushing this, I'm running some sanity tests. So far, this test sequence (adjusted to the fixed code) shows where force helps with older snapshots (I'll send separate email for showing how force helps active ABI-incompatible snapshots): Test 1: $ virsh snapshot-create-as dom snap # offline domain with one qcow2 disk $ virsh edit dom # add a second qcow2 disk $ virsh snapshot-revert dom snap # offline revert doesn't need force $ virsh dumpxml dom # sure enough, second disk is gone Whoops, this part of the test didn't quite work out, either. I need to revert to the snapshot domain prior to determining the list of disks to iterate over, so that we avoid calling qemu-img snapshot -a on the disk image that was not part of the snapshot. Likewise, snapshot-delete should call qemu-img snapshot -d only on the disk images involved in the snapshot. And here's what I have to squash in for test 1 to succeed as planned. Rather than squash in the fixed qemu-img iteration unreviewed into this ACK'd patch, I'll submit it as a separate patch. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Connection does not support host device enumeration
2011/10/1 Kaushal Shriyan kaushalshri...@gmail.com: 2011/9/29 Kaushal Shriyan kaushalshri...@gmail.com: 2011/9/27 Osier Yang jy...@redhat.com: 于 2011年09月27日 01:13, Kaushal Shriyan 写道: Hi, Kaushal I don't use CentOS, so don't known it, though I guess it's the intention or mistake of the packager. you can compile from the source code youself. # rpm -qi libvirt You will see the version you use currently, and get the same source tarball from http://libvirt.org/sources/ to compile. Of course, you can download newer source to compile, but it may also need newer dependency packages. So compiling from the same version is the easiest way. :-) If you really don't want to compile yourself, just file a bug to CentOS, guess the packger will help you do it. Regards, Osier Hi Osier Can you please point me to the rpm version which has hal and udev flags enabled by default for CentOS 5.6 ? Regards Kaushal On Thu, Sep 22, 2011 at 10:22 AM, Osier Yang jy...@redhat.com wrote: 于 2011年09月22日 12:46, Osier Yang 写道: 于 2011年09月22日 12:41, Kaushal Shriyan 写道: On Thu, Sep 22, 2011 at 9:41 AM, Osier Yang jy...@redhat.com wrote: 于 2011年09月22日 09:11, Kaushal Shriyan 写道: does not support host device enumeration Seems your libvirt is compiled without --with-hal and --with-udev Regards Osier Hi Osier Thanks for the reply. Please let me know how do i enable this feature since its a binary package installed on CentOS 5.6 x86_64 architecture. You need to re-compile from the source if that's true. http://libvirt.org/compiling.html To make sure get your wanted, specifying --with-hal/--with-udev explicitly with yes will help. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Hi Osier, Is there libudev-devel or udev-devel package available on CentOS 5.6 ? [root@~]# yum search udev-devel Loaded plugins: fastestmirror Loading mirror speeds from cached hostfile * base: centos.aol.in * extras: centos.aol.in * updates: centos.aol.in base | 1.1 kB 00:00 extras | 2.1 kB 00:00 updates | 1.9 kB 00:00 Warning: No matches found for: udev-devel No Matches found [root@~]# yum search libudev-devel Loaded plugins: fastestmirror Loading mirror speeds from cached hostfile * base: centos.aol.in * extras: centos.aol.in * updates: centos.aol.in Warning: No matches found for: libudev-devel No Matches found [root@~]# Please suggest further and also it seems like this package is not available in CentOS 5.6 Regards Kaushal Hi, Please suggest/comment on my earlier post to this mailing list. Regards Kaushal Hi Osier, Checking in again. Please suggest/comment on my earlier post to this mailing list. Regards, Kaushal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Qemu/KVM guest boots 2x slower with vhost_net
Hello all, for people in qemu-devel list, you might want to have a look at the previous thread about this topic, at http://www.spinics.net/lists/kvm/msg61537.html but I will try to recap here. I found that virtual machines in my host booted 2x slower (on average it's 2x slower, but probably some parts are at least 3x slower) under libvirt compared to manual qemu-kvm launch. With the help of Daniel I narrowed it down to the vhost_net presence (default active when launched by libvirt) i.e. with vhost_net, boot process is *UNIFORMLY* 2x slower. The problem is still reproducible on my systems but these are going to go to production soon and I am quite busy, I might not have many more days for testing left. Might be just next saturday and sunday for testing this problem, so if you can write here some of your suggestions by saturday that would be most appreciated. I have performed some benchmarks now, which I hadn't performed in the old thread: openssl speed -multi 2 rsa : (cpu benchmark) show no performance difference with or without vhost_net disk benchmarks : show no performance difference with or without vhost_net the disk benchmarks were: (both with cache=none and cache=writeback) dd streaming read dd streaming write fio 4k random read in all cases of cache=none, cache=writeback with host cache dropped before test, cache=writeback with all fio data in host cache (measures context switch) fio 4k random write So I couldn't reproduce the problem with any benchmark that came to my mind. But in the boot process this is very visible. I'll continue the description below before that, here are the System Specifications: --- Host is with kernel 3.0.3 and Qemu-KVM 0.14.1, both vanilla and compiled by me. Libvirt is the version in Ubuntu 11.04 Natty which is 0.8.8-1ubuntu6.5 . I didn't recompile this one VM disks are LVs of LVM on MD raid array. The problem shows identically on both cache=none and cache=writeback. Aio native. Physical CPUs are: dual westmere 6-core (12 cores total, + hyperthreading) 2 vCPUs per VM. All VMs are idle or off except the VM being tested. Guests are: - multiple Ubuntu 11.04 Natty 64bit with their 2.6.38-8-virtual kernel: very-minimal Ubuntu installs with deboostrap (not from the Ubuntu installer) - one Fedora Core 6 32bit with a 32bit 2.6.38-8-virtual kernel + initrd both taken from Ubuntu Natty 32bit (so I could have virtio). Standard install (except kernel replaced afterwards). Always static IP address in all guests --- All types of guests show this problem, but it is more visible in the FC6 guest because the boot process is MUCH longer than in the debootstrap-installed ubuntus. Please note that most of boot process, at least from a certain point onwards, appears to the eye uniformly 2x or 3x slower under vhost_net, and by boot process I mean, roughly, copying by hand from some screenshots: Loading default keymap Setting hostname Setting up LVM - no volume groups found checking ilesystems... clean ... remounting root filesystem in read-write mode mounting local filesystems enabling local filesystems quotas enabling /etc/fstab swaps INIT entering runlevel 3 entering non-interactive startup Starting sysstat: calling the system activity data collector (sadc) Starting background readahead ** starting from here it is everything, or almost everything, much slower Checking for hardware changes Bringing up loopback interface Bringing up interface eth0 starting system logger starting kernel logger starting irqbalance starting potmap starting nfs statd starting rpc idmapd starting system message bus mounting other filesystems starting PC/SC smart card daemon (pcscd) starint hidd ... can't open HIDP control socket : address familiy not supported by protocol (this is an error due to backporting a new ubuntu kernel to FC6) starting autofs: loading autofs4 starting automount starting acpi daemon starting hpiod starting hpssd starting cups starting sshd starting ntpd starting sendmail starting sm-client startingg console mouse services starting crond starting xfs starting anacron starting atd starting youm-updatesd starting Avahi daemon starting HAL daemon From the point I marked, onwards, most are services, i.e. daemons listening from sockets, so I have thought that maybe the binding to a socket could have been slower under vhost_net, but trying to put nc in listening with: nc -l 15000 is instantaneous, so I am not sure. The shutdown of FC6 with basically the same services as above which tear down, is *also* much slower on vhost_net. Thanks for any suggestions R. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1.5/2] snapshot: use qemu-img on disks in use at time of snapshot
Once we know which set of disks belong to a snapshot, reverting or deleting that snapshot should visit just those disks, rather than also visiting disks that were hot-plugged in the meantime or skipping disks that were hot-unplugged in the meantime. * src/qemu/qemu_domain.c (qemuDomainSnapshotForEachQcow2): Use snapshot domain details when available. Avoid NULL deref. --- Detected while actually testing patch 2/2 in the series. This fixes the issue, but is worth a separate review before I push the series. src/qemu/qemu_domain.c | 24 1 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 65f721a..85bebd6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1402,6 +1402,14 @@ qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, const char *qemuimgarg[] = { NULL, snapshot, NULL, NULL, NULL, NULL }; int i; bool skipped = false; +virDomainDefPtr def; + +/* Prefer action on the disks in use at the time the snapshot was + * created; but fall back to current definition if dealing with a + * snapshot created prior to libvirt 0.9.5. */ +def = snap-def-dom; +if (!def) +def = vm-def; qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) { @@ -1412,36 +1420,36 @@ qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, qemuimgarg[2] = op; qemuimgarg[3] = snap-def-name; -for (i = 0; i vm-def-ndisks; i++) { +for (i = 0; i def-ndisks; i++) { /* FIXME: we also need to handle LVM here */ /* FIXME: if we fail halfway through this loop, we are in an * inconsistent state. I'm not quite sure what to do about that */ -if (vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { -if (!vm-def-disks[i]-driverType || -STRNEQ(vm-def-disks[i]-driverType, qcow2)) { +if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { +if (!def-disks[i]-driverType || +STRNEQ(def-disks[i]-driverType, qcow2)) { if (try_all) { /* Continue on even in the face of error, since other * disks in this VM may have the same snapshot name. */ VIR_WARN(skipping snapshot action on %s, - vm-def-disks[i]-dst); + def-disks[i]-dst); skipped = true; continue; } qemuReportError(VIR_ERR_OPERATION_INVALID, _(Disk device '%s' does not support snapshotting), -vm-def-disks[i]-dst); +def-disks[i]-dst); return -1; } -qemuimgarg[4] = vm-def-disks[i]-src; +qemuimgarg[4] = def-disks[i]-src; if (virRun(qemuimgarg, NULL) 0) { if (try_all) { VIR_WARN(skipping snapshot action on %s, - vm-def-disks[i]-info.alias); + def-disks[i]-dst); skipped = true; continue; } -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Connection does not support host device enumeration
On Wed, Oct 05, 2011 at 04:10:07AM +0530, Kaushal Shriyan wrote: 2011/10/1 Kaushal Shriyan kaushalshri...@gmail.com: 2011/9/29 Kaushal Shriyan kaushalshri...@gmail.com: 2011/9/27 Osier Yang jy...@redhat.com: 于 2011年09月27日 01:13, Kaushal Shriyan 写道: Hi, Kaushal I don't use CentOS, so don't known it, though I guess it's the intention or mistake of the packager. you can compile from the source code youself. # rpm -qi libvirt You will see the version you use currently, and get the same source tarball from http://libvirt.org/sources/ to compile. Of course, you can download newer source to compile, but it may also need newer dependency packages. So compiling from the same version is the easiest way. :-) If you really don't want to compile yourself, just file a bug to CentOS, guess the packger will help you do it. Regards, Osier Hi Osier Can you please point me to the rpm version which has hal and udev flags enabled by default for CentOS 5.6 ? Regards Kaushal On Thu, Sep 22, 2011 at 10:22 AM, Osier Yang jy...@redhat.com wrote: 于 2011年09月22日 12:46, Osier Yang 写道: 于 2011年09月22日 12:41, Kaushal Shriyan 写道: On Thu, Sep 22, 2011 at 9:41 AM, Osier Yang jy...@redhat.com wrote: 于 2011年09月22日 09:11, Kaushal Shriyan 写道: does not support host device enumeration Seems your libvirt is compiled without --with-hal and --with-udev Regards Osier Hi Osier Thanks for the reply. Please let me know how do i enable this feature since its a binary package installed on CentOS 5.6 x86_64 architecture. You need to re-compile from the source if that's true. http://libvirt.org/compiling.html To make sure get your wanted, specifying --with-hal/--with-udev explicitly with yes will help. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Hi Osier, Is there libudev-devel or udev-devel package available on CentOS 5.6 ? [root@~]# yum search udev-devel Loaded plugins: fastestmirror Loading mirror speeds from cached hostfile * base: centos.aol.in * extras: centos.aol.in * updates: centos.aol.in base | 1.1 kB 00:00 extras | 2.1 kB 00:00 updates | 1.9 kB 00:00 Warning: No matches found for: udev-devel No Matches found [root@~]# yum search libudev-devel Loaded plugins: fastestmirror Loading mirror speeds from cached hostfile * base: centos.aol.in * extras: centos.aol.in * updates: centos.aol.in Warning: No matches found for: libudev-devel No Matches found [root@~]# Please suggest further and also it seems like this package is not available in CentOS 5.6 Regards Kaushal Hi, Please suggest/comment on my earlier post to this mailing list. Regards Kaushal Hi Osier, Checking in again. Please suggest/comment on my earlier post to this mailing list. You need to read his advice to you carefully: you need either hal or udev; you're using Centos 5.x, so you need hal. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: correct misspelled 'enospc' option, and only use for werror
On 10/04/2011 03:17 PM, Eric Blake wrote: On 10/04/2011 12:35 PM, Laine Stump wrote: This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=730909 When support for setting the qemu disk error policy to 'enospc' was added, it was inadvertantly as enospace. This patch corrects that on spelling and grammar s/inadvertantly/inadvertently spelled/ You have to appreciate the irony, though, right? :-) the qemu commandline (while retaining the enospace spelling for libvirt's XML. Also, while examining the qemu source, I found that enospc is not allowed for the read error policy, only for write error policy (makes sense). Since libvirt currently only has a single error policy setting, when enospace is selected, the read error policy is set to ignore. --- src/qemu/qemu_command.c | 22 ++ 1 files changed, 18 insertions(+), 4 deletions(-) ACK. Okay, I'm pushing this one without waiting for 2/2, though, as I haven't decided how to deal with some of the issues you brought up in your review. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list