Re: [libvirt] Effect of cluster size on read write performance
On Wed, Feb 22, 2012 at 12:36:00PM +0530, Pankaj Rawat wrote: Hi all The cluster -size increase write performance when the qcow2 image is not expanded , surely with increase in cluster_size the disk space used increases but that is not my concern. Can anyone tell what is the effect of cluster size on read and write performance on qcow2 image after expantion? Can you send this query to qemu-devel instead - they'll likely be able to give you a more accurate answer than folks here, since they are more familiar with these aspects of Qcow2 Regards, 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] Fwd: a question about sanlock
Hi Daniel, Thanks for your quick reply, from safety point of view, yes, libvirt should exit, but, your alternative solution is more friendly for users, however, I haven't any idea about this now, maybe, others want to do this, so forward it to libvirt-list. Thanks Regards, Alex - Forwarded Message - From: Daniel P. Berrange berra...@redhat.com To: Alex Jia a...@redhat.com Cc: libvirt-us...@redhat.com Sent: Wednesday, February 22, 2012 4:19:28 PM Subject: Re: [libvirt] a question about sanlock On Wed, Feb 22, 2012 at 02:04:09AM -0500, Alex Jia wrote: Hi Daniel, I got a question about lock manager, if I enable 'sanlock' in qemu.conf and uncomment 'auto_disk_leases = 1' in qemu-sanlock.conf then restart libvirtd service, libvirtd will be dead, I know I should also uncomment 'host_id = 1' in qemu-sanlock.conf, because I enable 'auto_disk_leases'. The question is the libvirtd must die due to a error users configuration? or could we give some warning information instead of libvirtd die? I think this is a safety issue - if someone is deploying libvirt in an scenario where they want locking, then we must be very careful not to accidentally run without locking. So if someone accidentally mis-configures one of their libvirtd instances to not have any host_id parameter, I felt the only safe thing todo was to exit. An alternative would be to allow libvirtd to start, but then make sure it refuses to start any guests. I'm happy to take patches for the latter if someone wants to... 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] Re: [PATCH] virsh: Enhance list command to ease creation of shell scripts
The 21/02/12, Eric Blake wrote: On 02/21/2012 09:05 AM, Peter Krempa wrote: @@ -950,77 +986,125 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } } -if (desc) { -vshPrintExtra(ctl, %-5s %-30s %-10s %s\n, _(Id), _(Name), _(State), _(Title)); -vshPrintExtra(ctl, ---\n); -} else { -vshPrintExtra(ctl, %-5s %-30s %s\n, _(Id), _(Name), _(State)); -vshPrintExtra(ctl, \n); The old style printed a variable-length line, depending on whether title was in the mix... +/* print table header in legacy mode */ +if (optTable) { +vshPrintExtra(ctl, %-5s %-30s %-10s, + _(Id), _(Name), _(State)); +if (optTitle) +vshPrintExtra(ctl, %-20s, _(Title)); + +vshPrintExtra(ctl, \n + ---\n); but your new version prints a fixed-width line as if title were always present. Not necessarily a show-stopper, but worth thinking about. BTW, I find that the %-ns format is not easy to parse from scripts. It would be easier with raw variable values and a dedicated separator like a tabulation. Human and scripts expectations are so... different! :-) -- Nicolas Sebrecht -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: Fix virFileAccessibleAs return path from parent
Despite documentation, if we do fork() parent always returns -1 even if file is accessible. Which is wrong obviously. --- src/util/util.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 3406b7b..04a0e79 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -724,8 +724,12 @@ virFileAccessibleAs(const char *path, int mode, return -1; } -errno = status; -return -1; +if (!status) { +errno = status; +return -1; +} + +return 0; } /* child. -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] vmware: implement domainXMLFromNative
--- src/vmware/vmware_driver.c | 31 +++ 1 files changed, 31 insertions(+), 0 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 56e9d2d..8f9d922 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -844,6 +844,36 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) return ret; } +static char * +vmwareDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, + const char *nativeConfig, + unsigned int flags) +{ +struct vmware_driver *driver = conn-privateData; +virVMXContext ctx; +virDomainDefPtr def = NULL; +char *xml = NULL; + +virCheckFlags(0, NULL); + +if (STRNEQ(nativeFormat, vmware-vmx)) { +vmwareError(VIR_ERR_INVALID_ARG, +_(Unsupported config format '%s'), nativeFormat); +return NULL; +} + +ctx.parseFileName = vmwareCopyVMXFileName; + +def = virVMXParseConfig(ctx, driver-caps, nativeConfig); + +if (def != NULL) +xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); + +virDomainDefFree(def); + +return xml; +} + static int vmwareNumDefinedDomains(virConnectPtr conn) { @@ -988,6 +1018,7 @@ static virDriver vmwareDriver = { .domainGetInfo = vmwareDomainGetInfo, /* 0.8.7 */ .domainGetState = vmwareDomainGetState, /* 0.9.2 */ .domainGetXMLDesc = vmwareDomainGetXMLDesc, /* 0.8.7 */ +.domainXMLFromNative = vmwareDomainXMLFromNative, /* 0.9.11 */ .listDefinedDomains = vmwareListDefinedDomains, /* 0.8.7 */ .numOfDefinedDomains = vmwareNumDefinedDomains, /* 0.8.7 */ .domainCreate = vmwareDomainCreate, /* 0.8.7 */ -- 1.7.9 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] VMware Workstation domainXMLFromNative support
These patches add domainXMLFromNative to the vmware driver and add support for more network configurations in vmx files Jean-Baptiste Rouault (2): vmware: implement domainXMLFromNative vmx: Better Workstation vmx handling src/vmware/vmware_driver.c | 31 +++ src/vmx/vmx.c | 29 ++--- 2 files changed, 49 insertions(+), 11 deletions(-) -- 1.7.9 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] vmx: Better Workstation vmx handling
This patch adds support for vmx files with empty networkName values (which is the case for vmx generated by Workstation). It also adds support for vmx containing NATed network interfaces. --- src/vmx/vmx.c | 29 ++--- 1 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 823d5df..3cc3b10 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2418,12 +2418,20 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) } /* vmx:networkName - def:data.bridge.brname */ -if ((connectionType == NULL || - STRCASEEQ(connectionType, bridged) || - STRCASEEQ(connectionType, custom)) -virVMXGetConfigString(conf, networkName_name, networkName, - false) 0) { -goto cleanup; +if (connectionType == NULL || +STRCASEEQ(connectionType, bridged) || +STRCASEEQ(connectionType, custom)) { +if (virVMXGetConfigString(conf, networkName_name, networkName, + true) 0) +goto cleanup; + +if (networkName == NULL) { +networkName = strdup(); +if (networkName == NULL) { +virReportOOMError(); +goto cleanup; +} +} } /* vmx:vnet - def:data.ifname */ @@ -2447,11 +2455,10 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) connectionType, connectionType_name); goto cleanup; } else if (STRCASEEQ(connectionType, nat)) { -/* FIXME */ -VMX_ERROR(VIR_ERR_INTERNAL_ERROR, - _(No yet handled value '%s' for VMX entry '%s'), - connectionType, connectionType_name); -goto cleanup; +(*def)-type = VIR_DOMAIN_NET_TYPE_USER; +(*def)-model = virtualDev; + +virtualDev = NULL; } else if (STRCASEEQ(connectionType, custom)) { (*def)-type = VIR_DOMAIN_NET_TYPE_BRIDGE; (*def)-model = virtualDev; -- 1.7.9 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] configure: Define program name if not found
AC_CHECK_PROG checks for program in given path. However, if it doesn't exists, [variable] is set to [value-if-not-found]. We don't want this to be the empty string in case of 'modprobe' and 'scrub' as we want to fallback to runtime detection. --- configure.ac |4 ++-- src/util/pci.c |5 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 4493b94..732f4fe 100644 --- a/configure.ac +++ b/configure.ac @@ -211,11 +211,11 @@ AC_PATH_PROG([UDEVADM], [udevadm], [], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([UDEVSETTLE], [udevsettle], [], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) -AC_PATH_PROG([MODPROBE], [modprobe], [], +AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) -AC_PATH_PROG([SCRUB], [scrub], [], +AC_PATH_PROG([SCRUB], [scrub], [scrub], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_DEFINE_UNQUOTED([DNSMASQ],[$DNSMASQ], diff --git a/src/util/pci.c b/src/util/pci.c index 633dcd8..c660e8d 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -40,11 +40,6 @@ #include virterror_internal.h #include virfile.h -/* avoid compilation breakage on some systems */ -#ifndef MODPROBE -# define MODPROBE modprobe -#endif - #define PCI_SYSFS /sys/bus/pci/ #define PCI_ID_LEN 10 /* */ #define PCI_ADDR_LEN 13 /* :XX:XX.X */ -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] vmx: Better Workstation vmx handling
2012/2/22 Jean-Baptiste Rouault jean-baptiste.roua...@diateam.net: This patch adds support for vmx files with empty networkName values (which is the case for vmx generated by Workstation). It also adds support for vmx containing NATed network interfaces. --- src/vmx/vmx.c | 29 ++--- 1 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 823d5df..3cc3b10 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2418,12 +2418,20 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) } /* vmx:networkName - def:data.bridge.brname */ - if ((connectionType == NULL || - STRCASEEQ(connectionType, bridged) || - STRCASEEQ(connectionType, custom)) - virVMXGetConfigString(conf, networkName_name, networkName, - false) 0) { - goto cleanup; + if (connectionType == NULL || + STRCASEEQ(connectionType, bridged) || + STRCASEEQ(connectionType, custom)) { + if (virVMXGetConfigString(conf, networkName_name, networkName, + true) 0) + goto cleanup; When networkName is NULL then there was no ethernet0.networkName entry in the VMX file at all. Setting it to an empty string here will result in an ethernet0.networkName = entry in the VMX file when the domain XML config is reformatted to VMX again. I'm not sure that this is correct. In general, a missing VMX entry means that the hypervisor should use the default value for this entry, but explicitly giving it an empty value will probably result in different behavior. As VIR_DOMAIN_NET_TYPE_BRIDGE requires a bridge name networkName cannot be NULL. So we need a special string to indicate that ethernet0.networkName is missing. This could be an empty string. To do this virVMXFormatEthernet needs to be changed to not add an ethernet0.networkName entry when def-data.bridge.brname is an empty string. A minor problem with this approach is that parsing and reformatting and VMX file with ethernet0.networkName = will result in removing this entry as is used as special value now. + if (networkName == NULL) { + networkName = strdup(); + if (networkName == NULL) { + virReportOOMError(); + goto cleanup; + } + } } /* vmx:vnet - def:data.ifname */ @@ -2447,11 +2455,10 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) connectionType, connectionType_name); goto cleanup; } else if (STRCASEEQ(connectionType, nat)) { - /* FIXME */ - VMX_ERROR(VIR_ERR_INTERNAL_ERROR, - _(No yet handled value '%s' for VMX entry '%s'), - connectionType, connectionType_name); - goto cleanup; + (*def)-type = VIR_DOMAIN_NET_TYPE_USER; + (*def)-model = virtualDev; + + virtualDev = NULL; } else if (STRCASEEQ(connectionType, custom)) { (*def)-type = VIR_DOMAIN_NET_TYPE_BRIDGE; (*def)-model = virtualDev; You're missing the counterpart in virVMXFormatEthernet to handle VIR_DOMAIN_NET_TYPE_USER. And this needs extra testcases in the vmx2xml and xml2vmx tests to cover this additions to the VMX parser before I can ACK it. A good start but it needs a v2. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] vmware: implement domainXMLFromNative
2012/2/22 Jean-Baptiste Rouault jean-baptiste.roua...@diateam.net: --- src/vmware/vmware_driver.c | 31 +++ 1 files changed, 31 insertions(+), 0 deletions(-) ACK. This one is good and independent from the second patch, so I push it now. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] vmx: Better Workstation vmx handling
2012/2/22 Matthias Bolte matthias.bo...@googlemail.com: And this needs extra testcases in the vmx2xml and xml2vmx tests to cover this additions to the VMX parser before I can ACK it. The test suite already has some actual in-the-wild ESX and GSX VMX config files. I suggest you add some actual VMX files for Workstation that cover the new aspects in this patch. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix virFileAccessibleAs return path from parent
On Wed, Feb 22, 2012 at 10:21:04 +0100, Michal Privoznik wrote: Despite documentation, if we do fork() parent always returns -1 even if file is accessible. Which is wrong obviously. --- src/util/util.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 3406b7b..04a0e79 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -724,8 +724,12 @@ virFileAccessibleAs(const char *path, int mode, return -1; } -errno = status; -return -1; +if (!status) { Are you sure about that '!' here? :-) +errno = status; +return -1; +} + +return 0; } /* child. ACK with the condition fixed. BTW, as currently written virFileAccessibleAs does not distinguish between a real error and inaccessible file. But I guess no caller needs this actually. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] configure: Define program name if not found
On Wed, Feb 22, 2012 at 11:28:33 +0100, Michal Privoznik wrote: AC_CHECK_PROG checks for program in given path. However, if it doesn't exists, [variable] is set to [value-if-not-found]. We don't want this to be the empty string in case of 'modprobe' and 'scrub' as we want to fallback to runtime detection. --- configure.ac |4 ++-- src/util/pci.c |5 - 2 files changed, 2 insertions(+), 7 deletions(-) ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix virFileAccessibleAs return path from parent
On 22.02.2012 12:06, Jiri Denemark wrote: On Wed, Feb 22, 2012 at 10:21:04 +0100, Michal Privoznik wrote: Despite documentation, if we do fork() parent always returns -1 even if file is accessible. Which is wrong obviously. --- src/util/util.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 3406b7b..04a0e79 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -724,8 +724,12 @@ virFileAccessibleAs(const char *path, int mode, return -1; } -errno = status; -return -1; +if (!status) { Are you sure about that '!' here? :-) +errno = status; +return -1; +} + +return 0; } /* child. ACK with the condition fixed. BTW, as currently written virFileAccessibleAs does not distinguish between a real error and inaccessible file. But I guess no caller needs this actually. Jirka Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] configure: Define program name if not found
On 22.02.2012 12:09, Jiri Denemark wrote: On Wed, Feb 22, 2012 at 11:28:33 +0100, Michal Privoznik wrote: AC_CHECK_PROG checks for program in given path. However, if it doesn't exists, [variable] is set to [value-if-not-found]. We don't want this to be the empty string in case of 'modprobe' and 'scrub' as we want to fallback to runtime detection. --- configure.ac |4 ++-- src/util/pci.c |5 - 2 files changed, 2 insertions(+), 7 deletions(-) ACK Jirka Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v5 12/12] suspend: add qmp events
Hi, Isn't this something where it is easier to omit first and add later once we have a use case, than to add up front only to find that no one cares? Yes. I'll drop it. cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Enhance list command to ease creation of shell scripts
On 02/21/2012 06:05 PM, Eric Blake wrote: On 02/21/2012 09:05 AM, Peter Krempa wrote: --- tools/virsh.c | 176 -- tools/virsh.pod | 20 ++- 2 files changed, 148 insertions(+), 48 deletions(-) Thanks for picking up on my suggestions, and implementing it so quickly! Well it makes it a lot easier to write shell scripts :) + +if ((optTable optName) || +(optTable optUUID) || +(optName optUUID)) { I think it might be easier to write this as: if (optTable + optName + optUUID 1) { That's elegant. -if (desc) { -vshPrintExtra(ctl, %-5s %-30s %-10s %s\n, _(Id), _(Name), _(State), _(Title)); -vshPrintExtra(ctl, ---\n); -} else { -vshPrintExtra(ctl, %-5s %-30s %s\n, _(Id), _(Name), _(State)); -vshPrintExtra(ctl, \n); The old style printed a variable-length line, depending on whether title was in the mix... It also broke the tests. +/* print table header in legacy mode */ +if (optTable) { +vshPrintExtra(ctl, %-5s %-30s %-10s, + _(Id), _(Name), _(State)); +if (optTitle) +vshPrintExtra(ctl, %-20s, _(Title)); + +vshPrintExtra(ctl, \n + ---\n); but your new version prints a fixed-width line as if title were always present. Not necessarily a show-stopper, but worth thinking about. ACK with the virsh.pod typo fixes. I fixed the typos and modified the default output to comply with the tests and pushed. Thanks. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name Add de-association handling for 802.1qbg (vepa) via lldpad netlink messages. Also adds the possibility to perform an association request without waiting for a confirmation. Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name --- src/qemu/qemu_migration.c|2 +- src/util/virnetdevmacvlan.c | 315 +- src/util/virnetdevvportprofile.c | 33 +++-- src/util/virnetdevvportprofile.h |3 +- 4 files changed, 339 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 12cfbde..31428f8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { net-mac, virDomainNetGetActualDirectDev(net), def-uuid, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0) + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0) goto err_exit; } last_good_net = i; diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 25d0846..b3e3325 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, passthrough) #if WITH_MACVTAP - # include stdint.h # include stdio.h # include errno.h @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include linux/if.h # include linux/if_tun.h +# include c-ctype.h + /* Older kernels lacked this enum value. */ # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU # define MACVLAN_MODE_PASSTHRU 8 @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include virfile.h # include virnetlink.h # include virnetdev.h +# include virpidfile.h + # define MACVTAP_NAME_PREFIX macvtap # define MACVTAP_NAME_PATTERN macvtap%d @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # define MACVLAN_NAME_PREFIX macvlan # define MACVLAN_NAME_PATTERN macvlan%d + /** * virNetDevMacVLanCreate: * @@ -445,6 +449,293 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU, }; +/* Struct to hold the state and configuration of a 802.1qbg port */ +struct virNetlinkCallbackData { + char cr_ifname[64]; + virNetDevVPortProfilePtr virtPortProfile; + const unsigned char *macaddress; + const char *linkdev; + const unsigned char *vmuuid; + enum virNetDevVPortProfileOp vmOp; + unsigned int linkState; +}; + +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; + +#define INSTANCE_STRLEN 36 + +static int instance2str(const unsigned char *p, char *dst, size_t size) +{ +if (dst size INSTANCE_STRLEN) { +snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x- +%02x%02x-%02x%02x%02x%02x%02x%02x, +p[0], p[1], p[2], p[3], +p[4], p[5], p[6], p[7], +p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); +return 0; +} +return -1; +} + +# define LLDPAD_PID_FILE /var/run/lldpad.pid +# define VIRIP_PID_FILE /var/run/virip.pid + +/** + * virNetDevMacVLanVPortProfileCallback: + * + * @msg: The buffer containing the received netlink message + * @length: The length of the received netlink message. + * @peer: The netling sockaddr containing the peer information + * @handeled: Contains information if the message has been replied to yet + * @opaque: Contains vital information regarding the associated vm an interface + * + * This function is called when a netlink message is received. The function + * reads the message and responds if it is pertinent to the running VMs + * network interface. + */ + +static void +virNetDevMacVLanVPortProfileCallback( unsigned char *msg, + int length, + struct sockaddr_nl *peer, + bool *handled, + void *opaque) +{ + struct nla_policy ifla_vf_policy[IFLA_VF_MAX + 1] = { +[IFLA_VF_MAC] = {.minlen = sizeof(struct ifla_vf_mac), +.maxlen = sizeof(struct ifla_vf_mac)}, +[IFLA_VF_VLAN] = {.minlen = sizeof(struct ifla_vf_vlan), +.maxlen = sizeof(struct ifla_vf_vlan)}, +}; + +struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = { +
[libvirt] [PATCH v5 1/2] util: Add netlink event handling to virnetlink.c
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name This code adds a netlink event interface to libvirt. It is based upon the event_poll code and makes use of it. An event is generated for each netlink message sent to the libvirt pid. Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name --- daemon/libvirtd.c|8 + src/libvirt_private.syms |6 + src/util/virnetlink.c| 438 +- src/util/virnetlink.h| 29 +++ 4 files changed, 476 insertions(+), 5 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b1b542b..ca8074d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -47,6 +47,7 @@ #include conf.h #include memory.h #include conf.h +#include virnetlink.h #include virnetserver.h #include threads.h #include remote.h @@ -1598,6 +1599,12 @@ int main(int argc, char **argv) { goto cleanup; } +/* Register the netlink event service */ +if (virNetlinkEventServiceStart() 0) { +ret = VIR_DAEMON_ERR_NETWORK; +goto cleanup; +} + /* Run event loop. */ virNetServerRun(srv); @@ -1607,6 +1614,7 @@ int main(int argc, char **argv) { 0, shutdown, NULL); cleanup: +virNetlinkEventServiceStop(); virNetServerProgramFree(remoteProgram); virNetServerProgramFree(qemuProgram); virNetServerClose(srv); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6ad36c..008470e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1258,6 +1258,12 @@ virNetDevVPortProfileOpTypeToString; #virnetlink.h virNetlinkCommand; +virNetlinkEventServiceIsRunning; +virNetlinkEventServiceStop; +virNetlinkEventServiceStart; +virNetlinkEventAddClient; +virNetlinkEventRemoveClient; + # virnetmessage.h diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index d03d171..ec4727e 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -35,7 +35,10 @@ #include sys/types.h #include virnetlink.h +#include logging.h #include memory.h +#include threads.h +#include virmacaddr.h #include virterror_internal.h #define VIR_FROM_THIS VIR_FROM_NET @@ -46,6 +49,50 @@ #define NETLINK_ACK_TIMEOUT_S 2 +#if defined(__linux__) defined(HAVE_LIBNL) +/* State for a single netlink event handle */ +struct virNetlinkEventHandle { +int watch; +virNetlinkEventHandleCallback cb; +void *opaque; +unsigned char macaddr[VIR_MAC_BUFLEN]; +int deleted; +}; + +typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate; +typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr; +struct _virNetlinkEventSrvPrivate { +/*Server*/ +virMutex lock; +int eventwatch; +int netlinkfd; +struct nl_handle *netlinknh; +/*Events*/ +int handled; +size_t handlesCount; +size_t handlesAlloc; +struct virNetlinkEventHandle *handles; +}; + +enum virNetlinkDeleteMode { +VIR_NETLINK_HANDLE_VALID, +VIR_NETLINK_HANDLE_DELETED, +}; + +/* Only have one event loop */ +//static struct virNetlinkEventLoop eventLoop; + +/* Unique ID for the next netlink watch to be registered */ +static int nextWatch = 1; + +/* Allocate extra slots for virEventPollHandle/virEventPollTimeout + records in this multiple */ +#define NETLINK_EVENT_ALLOC_EXTENT 10 + +static virNetlinkEventSrvPrivatePtr server = NULL; + +/* Function definitions */ + /** * virNetlinkCommand: * @nlmsg: pointer to netlink message @@ -58,7 +105,6 @@ * Returns 0 on success, -1 on error. In case of error, no response * buffer will be returned. */ -#if defined(__linux__) defined(HAVE_LIBNL) int virNetlinkCommand(struct nl_msg *nl_msg, unsigned char **respbuf, unsigned int *respbuflen, int nl_pid) @@ -89,7 +135,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, virReportSystemError(errno, %s, _(cannot connect to netlink socket)); rc = -1; -goto err_exit; +goto error; } nlmsg_set_dst(nl_msg, nladdr); @@ -101,7 +147,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, virReportSystemError(errno, %s, _(cannot send to netlink socket)); rc = -1; -goto err_exit; +goto error; } fd = nl_socket_get_fd(nlhandle); @@ -118,7 +164,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, virReportSystemError(ETIMEDOUT, %s, _(no valid netlink response was received)); rc = -1; -goto err_exit; +goto error; } *respbuflen = nl_recv(nlhandle, nladdr, respbuf, NULL); @@ -127,7 +173,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, %s, _(nl_recv failed)); rc = -1; } -err_exit: +error: if (rc == -1) { VIR_FREE(*respbuf); *respbuf = NULL; @@ -138,6 +184,323 @@ err_exit: return rc; }
[libvirt] [PATCH v5 0/2] util: Add netlink event handling code
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name This is the updated netlink event code based upon the updated virnetlink.[ch] files. Also the related code to virnetdevmacvlan is included as well as the related changes to the setup code. D. Herrendoerfer (2): util: Add netlink event handling to virnetlink.c Add de-association handling to macvlan code daemon/libvirtd.c|8 + src/libvirt_private.syms |6 + src/qemu/qemu_migration.c|2 +- src/util/virnetdevmacvlan.c | 315 +++- src/util/virnetdevvportprofile.c | 33 ++- src/util/virnetdevvportprofile.h |3 +- src/util/virnetlink.c| 438 +- src/util/virnetlink.h| 29 +++ 8 files changed, 815 insertions(+), 19 deletions(-) -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v5 12/12] suspend: add qmp events
On Wed, 22 Feb 2012 13:08:16 +0100 Gerd Hoffmann kra...@redhat.com wrote: Hi, Isn't this something where it is easier to omit first and add later once we have a use case, than to add up front only to find that no one cares? Yes. I'll drop it. Yes, I think it's better to drop it for now and add it back if really needed, when we'll know better how mngt apps are going to use this. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Migration failure related to EOF from Monitor
Hello All, I am trying to migrate a KVM guest while implementing the patches for PCI-Passthrough of SRIOV Vf's and I repeatedly see the following error: [root@c6100k cwd]# virsh migrate --live dibenchvm1 qemu+ssh://c6100l.uk.level5networks.com/system error: operation failed: migration job: unexpectedly failed I have turned on debugging in /etc/libvirt/libvirtd.conf and I see the following debug in /var/log/libvirt/libvirtd.log 10:37:26.632: 2449: error : qemuMonitorIO:584 : internal error End of file from monitor 10:37:26.632: 2449: debug : qemuMonitorIO:617 : Error on monitor internal error End of file from monitor 10:37:26.632: 2449: debug : virEventPollUpdateHandle:145 : Update handle w=21 e=12 10:37:26.632: 2449: debug : virEventPollInterruptLocked:676 : Skip interrupt, 1 -1497479264 10:37:26.632: 2449: debug : qemuMonitorIO:640 : Triggering EOF callback 10:37:26.632: 2449: debug : qemuProcessHandleMonitorEOF:125 : Received EOF on 0x7f588c003540 'dibenchvm1' 10:37:26.632: 2449: debug : qemuProcessStop:3271 : Shutting down VM 'dibenchvm1' pid=18739 migrated=0 10:37:26.632: 2449: debug : qemuMonitorClose:765 : mon=0x7f588c00c7f0 10:37:26.632: 2449: debug : virEventPollRemoveHandle:172 : Remove handle w=21 10:37:26.633: 2449: debug : virEventPollRemoveHandle:185 : mark delete 9 20 10:37:26.633: 2449: debug : virEventPollInterruptLocked:676 : Skip interrupt, 1 -1497479264 10:37:26.633: 2449: debug : qemuProcessKill:3217 : vm=dibenchvm1 pid=18739 gracefully=0 10:37:26.633: 2449: debug : qemuProcessAutoDestroyRemove:3736 : vm=dibenchvm1 uuid=a4452511-0f97-734a-dbcc-f4c66f821956 10:37:26.633: 2449: debug : virSecurityDACRestoreSecurityAllLabel:504 : Restoring security label on dibenchvm1 migrated=0 May I ask for suggestions on how I can debug the monitor and find out the reason why the qemu monitor dies? Many Thanks, Regards, Shradha Shah -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Forbid migration with cache != none
On Wed, Feb 22, 2012 at 14:58:31 +0800, Shu Ming wrote: On 2012-2-22 0:17, Jiri Denemark wrote: @@ -817,6 +817,27 @@ qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm, return true; } +static bool +qemuMigrationIsSafe(virDomainDefPtr def) +{ +int i; + +for (i = 0 ; i def-ndisks ; i++) { +virDomainDiskDefPtr disk = def-disks[i]; + +/* shared !readonly implies cache=none */ +if (disk-cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE how about disk write through flag here? This flag should also imply cache=none. i.e, VIR_DOMAIN_DISK_CACHE_WRITETHRU AFAIK, only VIR_DOMAIN_DISK_CACHE_DISABLE translates into cache=none unless qemu binary is old enough to only support cache=on|off Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration failure related to EOF from Monitor
On 22.02.2012 14:46, Shradha Shah wrote: Hello All, I am trying to migrate a KVM guest while implementing the patches for PCI-Passthrough of SRIOV Vf's and I repeatedly see the following error: [root@c6100k cwd]# virsh migrate --live dibenchvm1 qemu+ssh://c6100l.uk.level5networks.com/system error: operation failed: migration job: unexpectedly failed I have turned on debugging in /etc/libvirt/libvirtd.conf and I see the following debug in /var/log/libvirt/libvirtd.log 10:37:26.632: 2449: error : qemuMonitorIO:584 : internal error End of file from monitor 10:37:26.632: 2449: debug : qemuMonitorIO:617 : Error on monitor internal error End of file from monitor 10:37:26.632: 2449: debug : virEventPollUpdateHandle:145 : Update handle w=21 e=12 10:37:26.632: 2449: debug : virEventPollInterruptLocked:676 : Skip interrupt, 1 -1497479264 10:37:26.632: 2449: debug : qemuMonitorIO:640 : Triggering EOF callback 10:37:26.632: 2449: debug : qemuProcessHandleMonitorEOF:125 : Received EOF on 0x7f588c003540 'dibenchvm1' 10:37:26.632: 2449: debug : qemuProcessStop:3271 : Shutting down VM 'dibenchvm1' pid=18739 migrated=0 10:37:26.632: 2449: debug : qemuMonitorClose:765 : mon=0x7f588c00c7f0 10:37:26.632: 2449: debug : virEventPollRemoveHandle:172 : Remove handle w=21 10:37:26.633: 2449: debug : virEventPollRemoveHandle:185 : mark delete 9 20 10:37:26.633: 2449: debug : virEventPollInterruptLocked:676 : Skip interrupt, 1 -1497479264 10:37:26.633: 2449: debug : qemuProcessKill:3217 : vm=dibenchvm1 pid=18739 gracefully=0 10:37:26.633: 2449: debug : qemuProcessAutoDestroyRemove:3736 : vm=dibenchvm1 uuid=a4452511-0f97-734a-dbcc-f4c66f821956 10:37:26.633: 2449: debug : virSecurityDACRestoreSecurityAllLabel:504 : Restoring security label on dibenchvm1 migrated=0 May I ask for suggestions on how I can debug the monitor and find out the reason why the qemu monitor dies? I've seen this many times esp. when using upstream qemu. I mean in general when qemu dies during migration. Some info may be in domain log under /var/log/libvirt/qemu/domain.log; But if qemu dies there is not much libvirt can do to resurrect it. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] Add support for unsafe migration
This patch adds VIR_MIGRATE_UNSAFE flag for migration APIs and new VIR_ERR_MIGRATION_UNSAFE error code. The error code should be returned whenever migrating a domain is considered unsafe (e.g., it's configured in a way that does not ensure data integrity once it is migrated). VIR_MIGRATE_UNSAFE flag may be used to force migration even though it would normally be considered unsafe and forbidden. --- Notes: Version 2: - no change include/libvirt/libvirt.h.in |2 +- include/libvirt/virterror.h |1 + src/libvirt.c|4 src/util/virterror.c |6 ++ 4 files changed, 12 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 798ab07..e29df2a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -934,7 +934,7 @@ typedef enum { VIR_MIGRATE_CHANGE_PROTECTION = (1 8), /* protect for changing domain configuration through the * whole migration process; this will be used automatically * when supported */ - +VIR_MIGRATE_UNSAFE= (1 9), /* force migration even if it is considered unsafe */ } virDomainMigrateFlags; /* Domain migration. */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 9dbadfe..a3f9199 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -245,6 +245,7 @@ typedef enum { canceled/aborted by user */ VIR_ERR_AUTH_CANCELLED = 79,/* authentication cancelled */ VIR_ERR_NO_DOMAIN_METADATA = 80,/* The metadata is not present */ +VIR_ERR_MIGRATE_UNSAFE = 81,/* Migration is not safe */ } virErrorNumber; /** diff --git a/src/libvirt.c b/src/libvirt.c index 6294196..a3bd4f4 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5110,6 +5110,7 @@ virDomainMigrateDirect (virDomainPtr domain, * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5301,6 +5302,7 @@ error: * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5509,6 +5511,7 @@ error: * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. * * The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag. * If the VIR_MIGRATE_PEER2PEER flag is NOT set, the duri parameter @@ -5633,6 +5636,7 @@ error: * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. * * The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag. * diff --git a/src/util/virterror.c b/src/util/virterror.c index fb5ca6f..de60185 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1232,6 +1232,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _(metadata not found: %s); break; +case VIR_ERR_MIGRATE_UNSAFE: +if (!info) +errmsg = _(Unsafe migration); +else +errmsg = _(Unsafe migration: %s); +break; } return (errmsg); } -- 1.7.8.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] Forbid migration with cache != none
Migrating qemu domains with disks using cache != none is unsafe unless the disk images are stored on coherent clustered filesystem. Thus we forbid migrating such domains unless VIR_MIGRATE_UNSAFE flags is used. This series uses similar aproach to forbidding unsafe PCI passthrough or disk format probing when we forbade those by default with the possibility to force them. Domain configuration is only checked on source, which makes migrating affected domains from an old libvirt to the new one possible. Migrating back is impossible since destination libvirtd would complain about unknown flag (the flag is not filtered so it gets to the destination even though it's not really used there). However, users of unknown clustered filesystems now have to always pass the new flag to be able to migrate because libvirtd would think they are doing something unsafe. Perhaps we should provide a system wide (i.e., /etc/libvirt/qemu.conf) tunable which would disable cache mode checking for all domains at once? I was also wondering if we should rather use more specific name for both the error code and flag, such as VIR(_ERR)?_MIGRATE_UNSAFE_CACHE (or ...UNSAFE_DISK) in the case we find other unsafe conditions... Version 2: - add virStorageFileIsClusterFS as suggested by Dan B. Jiri Denemark (4): Add support for unsafe migration virsh: Add --unsafe option to migrate command Introduce virStorageFileIsClusterFS qemu: Forbid migration with cache != none include/libvirt/libvirt.h.in |2 +- include/libvirt/virterror.h |1 + src/libvirt.c|4 src/libvirt_private.syms |1 + src/qemu/qemu_driver.c |3 ++- src/qemu/qemu_migration.c| 39 +++ src/qemu/qemu_migration.h|6 -- src/util/storage_file.c | 10 ++ src/util/storage_file.h |1 + src/util/virterror.c |6 ++ tools/virsh.c|4 tools/virsh.pod | 10 +- 12 files changed, 78 insertions(+), 9 deletions(-) -- 1.7.8.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] caps: Improve error if passed an unknown arch
On 02/20/2012 05:55 PM, Eric Blake wrote: On 02/20/2012 11:54 AM, Cole Robinson wrote: Previously we would have: os type 'hvm' arch 'idontexist' combination is not supported Now we get No guest options available for arch 'idontexist' or if options available but guest OS type not applicable: No os type 'xen' available for arch 'x86_64' Looks nicer. --- src/conf/capabilities.c | 28 src/conf/capabilities.h |9 ++--- src/conf/domain_conf.c | 13 +++-- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index d44ce1b..542bf03 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -497,6 +497,26 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, return NULL; } +/** + * virCapabilitiesSupportsGuestArch: + * @caps: capabilities to query + * @arch: Architecture to search for (eg, 'i686', 'x86_64') + * + * Returns non-zero if the capabilities support the + * requested architecture + */ +extern int +virCapabilitiesSupportsGuestArch(virCapsPtr caps, + const char *arch) +{ +int i; +for (i = 0 ; i caps-nguests ; i++) { +if (STREQ(caps-guests[i]-arch.name, arch)) +return 1; +} +return 0; Given how you are using this, I'd rather see it with a return type of 'bool', and return true or false in the method. @@ -529,9 +549,9 @@ virCapabilitiesSupportsGuestOSType(virCapsPtr caps, * requested operating system type */ extern int -virCapabilitiesSupportsGuestArch(virCapsPtr caps, - const char *ostype, - const char *arch) +virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, + const char *ostype, + const char *arch) { Then again, you were just copying existing style, so maybe we should update this whole file to use bool where appropriate, which would imply doing it as a separate patch. So, I guess that means I've effectively given: ACK. Thanks, pushed (on Monday, just forgot to mail). - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] qemu: Forbid migration with cache != none
Migrating domains with disks using cache != none is unsafe unless the disk images are stored on coherent clustered filesystem. Thus we forbid migrating such domains unless VIR_MIGRATE_UNSAFE flags is used. --- Notes: Version 2: - use virStorageFileIsClusterFS src/qemu/qemu_driver.c|3 ++- src/qemu/qemu_migration.c | 39 +++ src/qemu/qemu_migration.h |6 -- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 717bdf1..63a0703 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8767,7 +8767,8 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto endjob; if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname, - cookieout, cookieoutlen))) + cookieout, cookieoutlen, + flags))) goto endjob; if ((flags VIR_MIGRATE_CHANGE_PROTECTION)) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0af494..09494d6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -45,6 +45,7 @@ #include virtime.h #include locking/domain_lock.h #include rpc/virnetsocket.h +#include storage_file.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -817,6 +818,29 @@ qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm, return true; } +static bool +qemuMigrationIsSafe(virDomainDefPtr def) +{ +int i; + +for (i = 0 ; i def-ndisks ; i++) { +virDomainDiskDefPtr disk = def-disks[i]; + +/* shared !readonly implies cache=none */ +if (disk-src +disk-cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE +(disk-cachemode || !disk-shared || disk-readonly) +virStorageFileIsClusterFS(disk-src) == 1) { +qemuReportError(VIR_ERR_MIGRATE_UNSAFE, %s, +_(Migration may lead to data corruption if disks + use cache != none)); +return false; +} +} + +return true; +} + /** qemuMigrationSetOffline * Pause domain for non-live migration. */ @@ -1010,7 +1034,8 @@ char *qemuMigrationBegin(struct qemud_driver *driver, const char *xmlin, const char *dname, char **cookieout, - int *cookieoutlen) + int *cookieoutlen, + unsigned long flags) { char *rv = NULL; qemuMigrationCookiePtr mig = NULL; @@ -1018,9 +1043,9 @@ char *qemuMigrationBegin(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm-privateData; VIR_DEBUG(driver=%p, vm=%p, xmlin=%s, dname=%s, - cookieout=%p, cookieoutlen=%p, + cookieout=%p, cookieoutlen=%p, flags=%lx, driver, vm, NULLSTR(xmlin), NULLSTR(dname), - cookieout, cookieoutlen); + cookieout, cookieoutlen, flags); /* Only set the phase if we are inside QEMU_ASYNC_JOB_MIGRATION_OUT. * Otherwise we will start the async job later in the perform phase losing @@ -1032,6 +1057,9 @@ char *qemuMigrationBegin(struct qemud_driver *driver, if (!qemuMigrationIsAllowed(driver, vm, NULL)) goto cleanup; +if (!(flags VIR_MIGRATE_UNSAFE) !qemuMigrationIsSafe(vm-def)) +goto cleanup; + if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) goto cleanup; @@ -2070,7 +2098,7 @@ static int doPeer2PeerMigrate3(struct qemud_driver *driver, * a single job. */ dom_xml = qemuMigrationBegin(driver, vm, xmlin, dname, - cookieout, cookieoutlen); + cookieout, cookieoutlen, flags); if (!dom_xml) goto cleanup; @@ -2354,6 +2382,9 @@ qemuMigrationPerformJob(struct qemud_driver *driver, if (!qemuMigrationIsAllowed(driver, vm, NULL)) goto cleanup; +if (!(flags VIR_MIGRATE_UNSAFE) !qemuMigrationIsSafe(vm-def)) +goto cleanup; + resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; if ((flags (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index f806ca1..41e4eac 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -35,7 +35,8 @@ VIR_MIGRATE_PAUSED | \ VIR_MIGRATE_NON_SHARED_DISK | \ VIR_MIGRATE_NON_SHARED_INC | \ - VIR_MIGRATE_CHANGE_PROTECTION) + VIR_MIGRATE_CHANGE_PROTECTION |\ + VIR_MIGRATE_UNSAFE) enum qemuMigrationJobPhase { QEMU_MIGRATION_PHASE_NONE = 0, @@ -81,7 +82,8 @@ char *qemuMigrationBegin(struct qemud_driver *driver, const char *xmlin, const char *dname, char
[libvirt] [PATCH v2 2/4] virsh: Add --unsafe option to migrate command
--- Notes: Version 2: - no change tools/virsh.c |4 tools/virsh.pod | 10 +- 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e712e53..3be86ed 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6819,6 +6819,7 @@ static const vshCmdOptDef opts_migrate[] = { {copy-storage-inc, VSH_OT_BOOL, 0, N_(migration with non-shared storage with incremental copy (same base image shared between source and destination))}, {change-protection, VSH_OT_BOOL, 0, N_(prevent any configuration changes to domain until migration ends))}, +{unsafe, VSH_OT_BOOL, 0, N_(force migration even if it may be unsafe)}, {verbose, VSH_OT_BOOL, 0, N_(display the progress of migration)}, {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, {desturi, VSH_OT_DATA, VSH_OFLAG_REQ, N_(connection URI of the destination host as seen from the client(normal migration) or source(p2p migration))}, @@ -6892,6 +6893,9 @@ doMigrate (void *opaque) if (vshCommandOptBool (cmd, change-protection)) flags |= VIR_MIGRATE_CHANGE_PROTECTION; +if (vshCommandOptBool(cmd, unsafe)) +flags |= VIR_MIGRATE_UNSAFE; + if (xmlfile virFileReadAll(xmlfile, 8192, xml) 0) { vshError(ctl, _(file '%s' doesn't exist), xmlfile); diff --git a/tools/virsh.pod b/tools/virsh.pod index f35244f..0f1ea91 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -781,7 +781,7 @@ type attribute for the domain element of XML. =item Bmigrate [I--live] [I--direct] [I--p2p [I--tunnelled]] [I--persistent] [I--undefinesource] [I--suspend] [I--copy-storage-all] -[I--copy-storage-inc] [I--change-protection] [I--verbose] +[I--copy-storage-inc] [I--change-protection] [I--unsafe] [I--verbose] Idomain-id Idesturi [Imigrateuri] [Idname] [I--timeout Bseconds] [I--xml Bfile] @@ -802,6 +802,14 @@ is implicitly enabled when supported by the hypervisor, but can be explicitly used to reject the migration if the hypervisor lacks change protection support. I--verbose displays the progress of migration. +In some cases libvirt may refuse to migrate the domain because doing so may +lead, e.g., to data corruption and thus the migration is considered unsafe. +For QEMU domain, this may happen if the domain uses disks without explicitly +setting cache mode to none. Migrating such domains is unsafe unless the disk +images are stored on coherent clustered filesystem, such as GFS2 or GPFS. If +you are sure the migration is safe or you just do not care, use I--unsafe +to force the migration. + The Idesturi is the connection URI of the destination host, and Imigrateuri is the migration URI, which usually can be omitted. Idname is used for renaming the domain to new name during migration, which -- 1.7.8.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration failure related to EOF from Monitor
On 2012-2-22 21:46, Shradha Shah wrote: Hello All, I am trying to migrate a KVM guest while implementing the patches for PCI-Passthrough of SRIOV Vf's and I repeatedly see the following error: [root@c6100k cwd]# virsh migrate --live dibenchvm1 qemu+ssh://c6100l.uk.level5networks.com/system error: operation failed: migration job: unexpectedly failed Are you sure your ssh setting in the destination host is correct? Maybe, you can try qemu+tcp to ignore ssh setting. I have turned on debugging in /etc/libvirt/libvirtd.conf and I see the following debug in /var/log/libvirt/libvirtd.log 10:37:26.632: 2449: error : qemuMonitorIO:584 : internal error End of file from monitor 10:37:26.632: 2449: debug : qemuMonitorIO:617 : Error on monitor internal error End of file from monitor 10:37:26.632: 2449: debug : virEventPollUpdateHandle:145 : Update handle w=21 e=12 10:37:26.632: 2449: debug : virEventPollInterruptLocked:676 : Skip interrupt, 1 -1497479264 10:37:26.632: 2449: debug : qemuMonitorIO:640 : Triggering EOF callback 10:37:26.632: 2449: debug : qemuProcessHandleMonitorEOF:125 : Received EOF on 0x7f588c003540 'dibenchvm1' 10:37:26.632: 2449: debug : qemuProcessStop:3271 : Shutting down VM 'dibenchvm1' pid=18739 migrated=0 10:37:26.632: 2449: debug : qemuMonitorClose:765 : mon=0x7f588c00c7f0 10:37:26.632: 2449: debug : virEventPollRemoveHandle:172 : Remove handle w=21 10:37:26.633: 2449: debug : virEventPollRemoveHandle:185 : mark delete 9 20 10:37:26.633: 2449: debug : virEventPollInterruptLocked:676 : Skip interrupt, 1 -1497479264 10:37:26.633: 2449: debug : qemuProcessKill:3217 : vm=dibenchvm1 pid=18739 gracefully=0 10:37:26.633: 2449: debug : qemuProcessAutoDestroyRemove:3736 : vm=dibenchvm1 uuid=a4452511-0f97-734a-dbcc-f4c66f821956 10:37:26.633: 2449: debug : virSecurityDACRestoreSecurityAllLabel:504 : Restoring security label on dibenchvm1 migrated=0 May I ask for suggestions on how I can debug the monitor and find out the reason why the qemu monitor dies? Many Thanks, Regards, Shradha Shah -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Shu Mingshum...@linux.vnet.ibm.com IBM China Systems and Technology Laboratory -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] Introduce virStorageFileIsClusterFS
--- Notes: Version 2: - new patch src/libvirt_private.syms |1 + src/util/storage_file.c | 10 ++ src/util/storage_file.h |1 + 3 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e3573f..18a24e4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1011,6 +1011,7 @@ virStorageFileFormatTypeToString; virStorageFileFreeMetadata; virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; +virStorageFileIsClusterFS; virStorageFileIsSharedFS; virStorageFileIsSharedFSType; virStorageFileProbeFormat; diff --git a/src/util/storage_file.c b/src/util/storage_file.c index a8661e3..cd1c142 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1063,3 +1063,13 @@ int virStorageFileIsSharedFS(const char *path) VIR_STORAGE_FILE_SHFS_OCFS | VIR_STORAGE_FILE_SHFS_AFS); } + +int virStorageFileIsClusterFS(const char *path) +{ +/* These are coherent cluster filesystems known to be safe for + * migration with cache != none + */ +return virStorageFileIsSharedFSType(path, +VIR_STORAGE_FILE_SHFS_GFS2 | +VIR_STORAGE_FILE_SHFS_OCFS); +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 96afb12..13d0e87 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -82,6 +82,7 @@ enum { }; int virStorageFileIsSharedFS(const char *path); +int virStorageFileIsClusterFS(const char *path); int virStorageFileIsSharedFSType(const char *path, int fstypes); -- 1.7.8.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] qemu: Forbid migration with cache != none
On Wed, Feb 22, 2012 at 03:51:11PM +0100, Jiri Denemark wrote: Migrating domains with disks using cache != none is unsafe unless the disk images are stored on coherent clustered filesystem. Thus we forbid migrating such domains unless VIR_MIGRATE_UNSAFE flags is used. --- Notes: Version 2: - use virStorageFileIsClusterFS src/qemu/qemu_driver.c|3 ++- src/qemu/qemu_migration.c | 39 +++ src/qemu/qemu_migration.h |6 -- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 717bdf1..63a0703 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8767,7 +8767,8 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto endjob; if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname, - cookieout, cookieoutlen))) + cookieout, cookieoutlen, + flags))) goto endjob; if ((flags VIR_MIGRATE_CHANGE_PROTECTION)) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0af494..09494d6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -45,6 +45,7 @@ #include virtime.h #include locking/domain_lock.h #include rpc/virnetsocket.h +#include storage_file.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -817,6 +818,29 @@ qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm, return true; } +static bool +qemuMigrationIsSafe(virDomainDefPtr def) +{ +int i; + +for (i = 0 ; i def-ndisks ; i++) { +virDomainDiskDefPtr disk = def-disks[i]; + +/* shared !readonly implies cache=none */ +if (disk-src +disk-cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE +(disk-cachemode || !disk-shared || disk-readonly) +virStorageFileIsClusterFS(disk-src) == 1) { Isn't this test reversed. ie, we want to deny migration if *not* a cluster FS, eg == 0 surely ? 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 v2 4/4] qemu: Forbid migration with cache != none
On Wed, Feb 22, 2012 at 14:57:00 +, Daniel P. Berrange wrote: On Wed, Feb 22, 2012 at 03:51:11PM +0100, Jiri Denemark wrote: Migrating domains with disks using cache != none is unsafe unless the disk images are stored on coherent clustered filesystem. Thus we forbid migrating such domains unless VIR_MIGRATE_UNSAFE flags is used. --- Notes: Version 2: - use virStorageFileIsClusterFS src/qemu/qemu_driver.c|3 ++- src/qemu/qemu_migration.c | 39 +++ src/qemu/qemu_migration.h |6 -- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 717bdf1..63a0703 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8767,7 +8767,8 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto endjob; if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname, - cookieout, cookieoutlen))) + cookieout, cookieoutlen, + flags))) goto endjob; if ((flags VIR_MIGRATE_CHANGE_PROTECTION)) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0af494..09494d6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -45,6 +45,7 @@ #include virtime.h #include locking/domain_lock.h #include rpc/virnetsocket.h +#include storage_file.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -817,6 +818,29 @@ qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm, return true; } +static bool +qemuMigrationIsSafe(virDomainDefPtr def) +{ +int i; + +for (i = 0 ; i def-ndisks ; i++) { +virDomainDiskDefPtr disk = def-disks[i]; + +/* shared !readonly implies cache=none */ +if (disk-src +disk-cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE +(disk-cachemode || !disk-shared || disk-readonly) +virStorageFileIsClusterFS(disk-src) == 1) { Isn't this test reversed. ie, we want to deny migration if *not* a cluster FS, eg == 0 surely ? Eh, sure. Thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [vdsm] non-TLS spice connections
On Tue, Feb 21, 2012 at 05:17:50PM +0100, Christophe Fergeau wrote: Hi, On Tue, Feb 21, 2012 at 06:09:06PM +0200, Dan Kenigsberg wrote: Please note Bug 788092 - VDSM: Disable vdsm's ssl'ability influence spice ssl'ability and prevent VM from starting https://bugzilla.redhat.com/show_bug.cgi?id=788092#c1 Could it be that you have ssl=false in your vdsm.conf? I've got ssl=true in my vdsm.conf, and the VM starts properly (qemu is running after I start it in the web interface) so it's probably a slightly different issue I'm hitting. I understand that the issue appears to be a former Vdsm configuring listen_tls=0 in qemu.conf, but Vdsm being asked to start a VM with secure spice channels. From Vdsm's point of view, it would be nice if libvirt/qemu protected us from this situation, by not letting such a VM to start. But as I think of this further, I'm not sure - if a iron-made computer has a broken screen, I would still expect it to boot and answer the network. Would it similarly make sense for a VM to start when its qxl device is inaccessible to the world? Anyway, I believe that once Vdsm stops messing with spice's listen_tls just because Vdsm has ssl=false, the problem at hand would disappear. Dan. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt-guests: Enable parallel operations and improve error handling
On 02/20/2012 06:32 AM, Peter Krempa wrote: This patch modifies the libvirt-guests init script to enable parallel shiutdown on guest machines and gets rid of error messages if the client is unable connect to the URI specified in the config file. Resuming where I left off (and seeing as how you took my suggestions for improving 'virsh list', we'll need a v2 anyway), +eval_gettext Running guests on \$uri URI: -list=$(list_guests $uri) +list=$(list_guests $uri all) Yeah, I guess it makes sense to list all running guests, even the transient ones that we can't save, if only for log purposes to show that we knew we were losing the transient ones. Hmm, I just remembered, a while ago there was a PoC patch to allow migration on shutdown, as an alternative to managed-save - and that would work for transient guests! We need to look at reviving that patch, and figuring out how it would interact with this patch. https://www.redhat.com/archives/libvir-list/2011-November/msg00229.html @@ -292,13 +444,39 @@ stop() { eval_gettext Shutting down guests on \$uri URI...; echo fi -for guest in $list; do -if $suspending; then -suspend_guest $uri $guest -else -shutdown_guest $uri $guest -fi -done +if [ $PARALLEL_SHUTDOWN -gt 1 ] + ! $suspending; then +on_shutdown= +timeout=$SHUTDOWN_TIMEOUT +while [ $(set_count $on_shutdown) -gt 0 ] || + [ $(set_count $list) -gt 0 ]; do $(set_count ...) forks, but if all you are doing is checking for a non-empty set (count -gt 0), then it is faster to do: while [ -n $on_shutdown ] || [ -n $list ]; do +while [ $(set_count $on_shutdown) -lt $PARALLEL_SHUTDOWN ] whereas this use of set_count really is needed. + [ $(set_count $list) -gt 0 ]; do +domain=$(set_head $list) +shutdown_guest_async $uri $domain +on_shutdown=$(set_add $domain $on_shutdown); +list=$(set_remove $domain $list); +done +sleep 1 +timeout=$((timeout - 1)) My earlier patch mentioned that it might be slightly more portable to use $(($timeout - 1)) (that is, $timeout instead of timeout); but in looking further, I see that this is just copy-paste from existing code, and no one has complained about it not working on dash, so no worries about it after all. The parallel shutdown looks reasonable, but it's a lot of code to be embedding into the stop function; maybe it's worth factoring into a shutdown_guest_parallel function? +++ b/tools/libvirt-guests.sysconf @@ -10,7 +10,8 @@ # libvirtd #ON_BOOT=start -# number of seconds to wait between each guest start +# number of seconds to wait between each guest start. Set to 0 to allow parallel +# startup. #START_DELAY=0 # action taken on host shutdown @@ -23,7 +24,12 @@ # value suitable for your guests. #ON_SHUTDOWN=suspend -# number of seconds we're willing to wait for a guest to shut down +# If set to non-zero, shutdown will suspend domains concurently. Number of domains s/concurently/concurrently/ +# on shutdown at any time will not exceed number set in this variable. +#PARALLEL_SHUTDOWN=0 + +# number of seconds we're willing to wait for a guest to shut down. If parallel +# shutdown is enabled, this timeout applies as a timeout for shutting down all guests. #SHUTDOWN_TIMEOUT=0 # If non-zero, try to bypass the file system cache when saving and Looking forward to v2. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl-Sys-Virt][PATCH] de-mortalize the return value from hv_delete
when calling block_stats() like $dom-block_stats(), it will reprot errors: Attempt to free unreferenced scalar: SV 0x2598498, Perl interpreter: 0x11a0010. Attempt to free unreferenced scalar: SV 0x258c498, Perl interpreter: 0x11a0010. This patch fix it. --- Virt.xs |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Virt.xs b/Virt.xs index e5d8438..9ef9715 100644 --- a/Virt.xs +++ b/Virt.xs @@ -3358,6 +3358,7 @@ block_stats(dom, path, flags=0) field = flush_reqs; if (field) { SV *val = hv_delete(RETVAL, params[i].field, strlen(params[i].field), 0); + SvREFCNT_inc(val); (void)hv_store(RETVAL, field, strlen(field), val, 0); } } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [perl-Sys-Virt][PATCH] de-mortalize the return value from hv_delete
On Thu, Feb 23, 2012 at 02:14:17AM +0800, Guannan Ren wrote: when calling block_stats() like $dom-block_stats(), it will reprot errors: Attempt to free unreferenced scalar: SV 0x2598498, Perl interpreter: 0x11a0010. Attempt to free unreferenced scalar: SV 0x258c498, Perl interpreter: 0x11a0010. This patch fix it. --- Virt.xs |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Virt.xs b/Virt.xs index e5d8438..9ef9715 100644 --- a/Virt.xs +++ b/Virt.xs @@ -3358,6 +3358,7 @@ block_stats(dom, path, flags=0) field = flush_reqs; if (field) { SV *val = hv_delete(RETVAL, params[i].field, strlen(params[i].field), 0); + SvREFCNT_inc(val); (void)hv_store(RETVAL, field, strlen(field), val, 0); } } 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: nicer error message on failed graceful destroy
https://bugzilla.redhat.com/show_bug.cgi?id=795656 mentions that a graceful destroy request can time out, meaning that the error message is user-visible and should be more appropriate than just internal error. * src/qemu/qemu_driver.c (qemuDomainDestroyFlags): Swap error type. --- src/qemu/qemu_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 717bdf1..c7ca881 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1792,7 +1792,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, */ if (flags VIR_DOMAIN_DESTROY_GRACEFUL) { if (qemuProcessKill(driver, vm, 0) 0) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +qemuReportError(VIR_ERR_OPERATION_FAILED, %s, _(failed to kill qemu process with SIGTERM)); goto cleanup; } -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Forbid migration with cache != none
On 02/22/2012 07:51 AM, Jiri Denemark wrote: I was also wondering if we should rather use more specific name for both the error code and flag, such as VIR(_ERR)?_MIGRATE_UNSAFE_CACHE (or ...UNSAFE_DISK) in the case we find other unsafe conditions... I think that if we ever have sub-categories of unsafe operations, where we want the user to pass varying flags to allow one but not the other sub-category, then we could do: VIR_ERR_MIGRATION_UNSAFE_CACHE = 19, VIR_ERR_MIGRATION_UNSAFE_DISK = 110, VIR_ERR_MIGRATION_UNSAFE = (VIR_ERR_MIGRATION_UNSAFE_CACHE|VIR_ERR_MIGRATION_UNSAFE_DISK) that is, make the current generic name remain generic by having it cover multiple bits, while the specific bits control the sub-options. But for now, I'm fine with just a single, shorter, name. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] Add support for unsafe migration
On 02/22/2012 07:51 AM, Jiri Denemark wrote: This patch adds VIR_MIGRATE_UNSAFE flag for migration APIs and new VIR_ERR_MIGRATION_UNSAFE error code. The error code should be returned whenever migrating a domain is considered unsafe (e.g., it's configured in a way that does not ensure data integrity once it is migrated). VIR_MIGRATE_UNSAFE flag may be used to force migration even though it would normally be considered unsafe and forbidden. ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] virsh: Add --unsafe option to migrate command
On 02/22/2012 07:51 AM, Jiri Denemark wrote: --- Notes: Version 2: - no change tools/virsh.c |4 tools/virsh.pod | 10 +- 2 files changed, 13 insertions(+), 1 deletions(-) @@ -802,6 +802,14 @@ is implicitly enabled when supported by the hypervisor, but can be explicitly used to reject the migration if the hypervisor lacks change protection support. I--verbose displays the progress of migration. +In some cases libvirt may refuse to migrate the domain because doing so may +lead, e.g., to data corruption and thus the migration is considered unsafe. That doesn't flow very well. How about: because doing so may lead to potential problems such as data corruption, and thus the migration is considered unsafe. +For QEMU domain, this may happen if the domain uses disks without explicitly +setting cache mode to none. Migrating such domains is unsafe unless the disk +images are stored on coherent clustered filesystem, such as GFS2 or GPFS. If +you are sure the migration is safe or you just do not care, use I--unsafe +to force the migration. ACK with the wording tweak. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] Introduce virStorageFileIsClusterFS
On 02/22/2012 07:51 AM, Jiri Denemark wrote: --- Notes: Version 2: - new patch ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf/default.cfg: Update Fedora download URLs
libvirt TCK is currently broke since it can't download from the old alias download.fedora.redhat.com, since that alias was removed and people really should update their download links. Point to the new dl.fedoraproject.org URLs. Signed-off-by: Lucas Meneghel Rodrigues l...@redhat.com --- conf/default.cfg |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/conf/default.cfg b/conf/default.cfg index cb70149..a303d88 100644 --- a/conf/default.cfg +++ b/conf/default.cfg @@ -69,8 +69,8 @@ kernels = ( hvm xen ) - kernel = http://download.fedora.redhat.com/pub/fedora/linux/releases/15/Fedora/i386/os/images/pxeboot/vmlinuz-PAE - initrd = http://download.fedora.redhat.com/pub/fedora/linux/releases/15/Fedora/i386/os/images/pxeboot/initrd-PAE.img + kernel = http://dl.fedoraproject.org/pub/fedora/linux/releases/15/Fedora/i386/os/images/pxeboot/vmlinuz-PAE + initrd = http://dl.fedoraproject.org/pub/fedora/linux/releases/15/Fedora/i386/os/images/pxeboot/initrd-PAE.img } # Fedora 15 x86_64 has pv_ops, so one kernel can do both Xen and KVM guests here { @@ -79,8 +79,8 @@ kernels = ( hvm xen ) - kernel = http://download.fedora.redhat.com/pub/fedora/linux/releases/15/Fedora/x86_64/os/images/pxeboot/vmlinuz - initrd = http://download.fedora.redhat.com/pub/fedora/linux/releases/15/Fedora/x86_64/os/images/pxeboot/initrd.img + kernel = http://dl.fedoraproject.org/pub/fedora/linux/releases/15/Fedora/x86_64/os/images/pxeboot/vmlinuz + initrd = http://dl.fedoraproject.org/pub/fedora/linux/releases/15/Fedora/x86_64/os/images/pxeboot/initrd.img } # User mode linux i686 needs custom kernel + root filesystem { -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf/default.cfg: Update Fedora download URLs
On Wed, Feb 22, 2012 at 05:12:29PM -0200, Lucas Meneghel Rodrigues wrote: libvirt TCK is currently broke since it can't download from the old alias download.fedora.redhat.com, since that alias was removed and people really should update their download links. Point to the new dl.fedoraproject.org URLs. Signed-off-by: Lucas Meneghel Rodrigues l...@redhat.com --- conf/default.cfg |8 1 files changed, 4 insertions(+), 4 deletions(-) ACK pushed. 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 v2 4/4] qemu: Forbid migration with cache != none
On 02/22/2012 07:51 AM, Jiri Denemark wrote: Migrating domains with disks using cache != none is unsafe unless the disk images are stored on coherent clustered filesystem. Thus we forbid migrating such domains unless VIR_MIGRATE_UNSAFE flags is used. --- Notes: Version 2: - use virStorageFileIsClusterFS src/qemu/qemu_driver.c|3 ++- src/qemu/qemu_migration.c | 39 +++ src/qemu/qemu_migration.h |6 -- 3 files changed, 41 insertions(+), 7 deletions(-) +static bool +qemuMigrationIsSafe(virDomainDefPtr def) +{ +int i; + +for (i = 0 ; i def-ndisks ; i++) { +virDomainDiskDefPtr disk = def-disks[i]; + +/* shared !readonly implies cache=none */ +if (disk-src +disk-cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE +(disk-cachemode || !disk-shared || disk-readonly) +virStorageFileIsClusterFS(disk-src) == 1) { Other than Dan's comment about the logic bug here, ACK. Since the check for safety is only on the source, and the destination doesn't care, is there a way to add a driver feature flag, and add logic to libvirt.c to mask the VIR_MIGRATE_UNSAFE flag from the destination if it does not support the feature, similar to how we handled VIR_MIGRATE_CHANGE_PROTECTION via the VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION feature, as another example of a source-only flag? Of course, this would be a followup patch, if we decide it is worth allowing an unsafe migration from 1.9.11 back to 1.9.10 (the upgrade migration from 1.9.10 to 1.9.11 will be unsafe automatically, because we weren't checking in 1.9.10). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/7] virsh: add support for VIR_DOMAIN_CONSOLE_* flags
On 02/06/2012 06:50 AM, Peter Krempa wrote: This patch adds support for the newly introduced VIR_DOMAIN_CONSOLE_FORCE and VIR_DOMAIN_CONSOLE_SAFE flags. The console command now has an optional parameter --force that specifies that the user wants to forcibly interrupt an ongoing console session and create a new one. Flag --safe requests that the console should be opened only if the hypervisor driver supports safe console handling. The behaviour to this point was that the daemon opened two streams to the console, that competed for data from the pipe, and the result was that both of the consoles ended up scrambled. This patch doesn't modify operation of other commands dealing with console connections (start, create) as those open connections to newly started domains making it virtualy impossible for another client to race s/virtualy/virtually/ ACK. And sorry for dragging this review on for so long - I promise to finish it today ;) -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/7] fdstream: Emit stream abort callback even if poll() doesnt.
On 02/06/2012 06:50 AM, Peter Krempa wrote: This patch causes the fdstream driver to call the stream event callback if virStreamAbort() is issued on a stream using this driver. This prohibited to abort streams from the daemon, as the daemon remote handler installs a callback to watch for stream errors as the only mean of detecting changes in the stream. That sentence didn't parse well for me; are you trying to say: A remote handler for a stream can only detect changes via stream events, so this event callback is necessary in order to enable a daemon to abort a stream in such a way that the client will see the change. * src/fdstream.c: - modify close function to call stream event callback --- src/fdstream.c | 56 ++-- 1 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index 841f979..35f5135 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -1,5 +1,5 @@ /* - * fdstream.h: generic streams impl for file descriptors + * fdstream.c: generic streams impl for file descriptors Cute typo fix... * * Copyright (C) 2009-2011 Red Hat, Inc. but it made me notice this. It's now 2012. If you use emacs, you can add this to your ~/.emacs to auto-update copyright in any file you touch; here's what I use: (require 'copyright) (defun my-copyright-update (optional arg) My improvements to `copyright-update'. (interactive *P) (and (not (eq major-mode 'fundamental-mode)) (copyright-update arg)) nil) (add-hook 'before-save-hook 'my-copyright-update) [hmm - should we follow the lead of GNU programs that just globally update the copyright year on all files when a new year rolls around? but that's a question to ask Red Hat legal] @@ -120,6 +126,7 @@ static int virFDStreamUpdateCallback(virStreamPtr stream, int events) } virEventUpdateHandle(fdst-watch, events); +fdst-events = events; On update, you leave fdst-abortCallbackCalled unchanged, ret = 0; @@ -214,6 +221,8 @@ virFDStreamAddCallback(virStreamPtr st, fdst-cb = cb; fdst-opaque = opaque; fdst-ff = ff; +fdst-events = events; +fdst-abortCallbackCalled = false; but on Add, you always clear it. Any significance to this difference? +/* aborting the stream, ensure the callback is called if it's + * registered for stream error event */ +if (streamAbort +fdst-cb +(fdst-events (VIR_STREAM_EVENT_READABLE | + VIR_STREAM_EVENT_WRITABLE))) { +/* don't enter this function accidentaly from the callback again */ s/accidentaly/accidentally/ +if (fdst-abortCallbackCalled) { +virMutexUnlock(fdst-lock); +return 0; +} + +fdst-abortCallbackCalled = true; +fdst-abortCallbackDispatching = true; +virMutexUnlock(fdst-lock); + +/* call failure callback, poll does report nothing on closed fd */ s/does report/reports/ +(fdst-cb)(st, VIR_STREAM_EVENT_ERROR, fdst-opaque); You need to cache fdst-cb and fdst-opaque before dropping locks, since otherwise you could have a race with someone else updating the callback to a different value. I'd still feel more comfortable with a review from Dan, but since that seems to be long in coming, you have my ACK if you can fix the problems pointed out above. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt TCK wrapper for autotest review
Hi guys, I was here looking at the autotest wrapper for libvirt TCK and then decided to work on it, as I had the review fresh on my mind. Things that I've worked on: * Fixed some download links, that were already sent to upstream tck and applied (thanks Dan Berrange) * Instead of making all tests output to the same DEBUG log, make them output to separate .tap files on the results directory * Run all tests available for a given item, rather than stopping the test on the first failure * removed capitalization on the wrapper name, since it's project policy * Use os.environ, and some features of the subcommand execution API to execute the tests * Remove usages of error.JobError, as the problems there are more error.TestError, since they are restricted to the libvirt_tck test, not the entire job (in autotest, a job can do more stuff than just a sequence of job.runtest() calls). * Made the error messages more descriptive, with info of all failed tests So, the current output of the tests is like this: $ sudo client/bin/autotest run libvirt_tck 18:29:27 INFO | Writing results to /home/lmr/Code/autotest.lmr/client/results/default 18:29:27 INFO | START timestamp=1329942567 localtime=Feb 22 18:29:27 18:29:27 INFO | START libvirt_tck.domain libvirt_tck.domain timestamp=1329942567 localtime=Feb 22 18:29:27 18:30:19 ERROR| child process failed 18:30:19 INFO | FAIL libvirt_tck.domain libvirt_tck.domain timestamp=1329942619 localtime=Feb 22 18:30:19 FAIL: ['120-disks-stats.t', '205-disk-hotplug-ordering.t'] 18:30:19 INFO | END FAIL libvirt_tck.domain libvirt_tck.domain timestamp=1329942619 localtime=Feb 22 18:30:19 18:30:19 INFO | START libvirt_tck.hooks libvirt_tck.hooks timestamp=1329942619 localtime=Feb 22 18:30:19 18:30:19 ERROR| child process failed 18:30:19 INFO | FAIL libvirt_tck.hooks libvirt_tck.hooks timestamp=1329942619 localtime=Feb 22 18:30:19 FAIL: ['051-daemon-hook.t', '052-domain-hook.t'] 18:30:19 INFO | END FAIL libvirt_tck.hooks libvirt_tck.hooks timestamp=1329942619 localtime=Feb 22 18:30:19 18:30:19 INFO | START libvirt_tck.networks libvirt_tck.networks timestamp=1329942619 localtime=Feb 22 18:30:19 18:30:28 INFO | GOOD libvirt_tck.networks libvirt_tck.networks timestamp=1329942628 localtime=Feb 22 18:30:28 completed successfully 18:30:28 INFO | END GOOD libvirt_tck.networks libvirt_tck.networks timestamp=1329942628 localtime=Feb 22 18:30:28 18:30:28 INFO | START libvirt_tck.nwfilter libvirt_tck.nwfilter timestamp=1329942628 localtime=Feb 22 18:30:28 18:30:32 ERROR| child process failed 18:30:32 INFO | FAIL libvirt_tck.nwfilter libvirt_tck.nwfilter timestamp=1329942632 localtime=Feb 22 18:30:32 FAIL: ['090-install-image.t', '100-ping-still-working.t', '210-no-mac-spoofing.t', '220-no-ip-spoofing.t', '230-no-mac-broadcast.t', '240-no-arp-spoofing.t', '300-vsitype.t'] 18:30:32 INFO | END FAIL libvirt_tck.nwfilter libvirt_tck.nwfilter timestamp=1329942632 localtime=Feb 22 18:30:32 18:30:32 INFO | START libvirt_tck.qemu libvirt_tck.qemu timestamp=1329942632 localtime=Feb 22 18:30:32 18:30:40 ERROR| child process failed 18:30:40 INFO | FAIL libvirt_tck.qemu libvirt_tck.qemu timestamp=1329942640 localtime=Feb 22 18:30:40 FAIL: ['205-qcow2-double-backing-file.t'] 18:30:40 INFO | END FAIL libvirt_tck.qemu libvirt_tck.qemu timestamp=1329942640 localtime=Feb 22 18:30:40 18:30:40 INFO | START libvirt_tck.selinux libvirt_tck.selinux timestamp=1329942640 localtime=Feb 22 18:30:40 18:30:49 ERROR| child process failed 18:30:49 INFO | FAIL libvirt_tck.selinux libvirt_tck.selinux timestamp=1329942649 localtime=Feb 22 18:30:49 FAIL: ['055-dynamic-base-label.t', '100-static-relabel-no.t'] 18:30:49 INFO | END FAIL libvirt_tck.selinux libvirt_tck.selinux timestamp=1329942649 localtime=Feb 22 18:30:49 18:30:49 INFO | START libvirt_tck.storage libvirt_tck.storage timestamp=1329942649 localtime=Feb 22 18:30:49 18:31:24 INFO | GOOD libvirt_tck.storage libvirt_tck.storage timestamp=1329942684 localtime=Feb 22 18:31:24 completed successfully 18:31:24 INFO | END GOOD libvirt_tck.storage libvirt_tck.storage timestamp=1329942684 localtime=Feb 22 18:31:24 18:31:24 INFO | END GOOD timestamp=1329942684 localtime=Feb 22 18:31:24 As time allows, I might take a look at the failures and help with libvirt_tck. I've combined all modifications to a single, self contained commit. Also, as the work is self contained, it could be very very easily rebased to the latest upstream tree. I've updated my personal repo and sent a github pull request that you guys can see and review: https://github.com/autotest/autotest/pull/192 I'm still not going to merge this to the upstream tree just yet, since I'd like to hear some feedback from you guys. As for developing together with autotest, I guess you can easily use the clone you have on libvirt now, and on your working directory you can add the following remote: [remote
Re: [libvirt] [PATCH v4 5/7] fdstream: Add internal callback on stream close
On 02/06/2012 06:50 AM, Peter Krempa wrote: This patch adds another callback to a FDstream object. The original callback is used by the daemon stream driver to handle events. This callback is called if and only if the stream is about to be closed. This might be used to handle cleanup steps after a fdstream exits. This will be used later on in ensuring mutualy exclusive access to consoles. s/mutualy/mutually/ * src/fdstream.c: - emit the callback, when stream is being closed - add data structures needed to handle the callback - add function to register callback * src/fdstream.h: - define function prototypes for the callback --- src/fdstream.c | 39 +-- src/fdstream.h | 11 +++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index 35f5135..192a7c4 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -67,6 +67,12 @@ struct virFDStreamData { bool abortCallbackCalled; bool abortCallbackDispatching; +/* internal callback, as the regular one (from generic streams) gets + * eaten up by the server stream driver */ +virFDStreamInternalCloseCb icbCb; +virFDStreamInternalCloseCbFreeOpaque icbFreeOpaque; +void *icbOpaque; + virMutex lock; }; @@ -232,6 +238,7 @@ cleanup: return ret; } + static int This whitespace change can be dropped, or moved to the patch that introduces virFDStreamCloseInt. virFDStreamCloseInt(virStreamPtr st, bool streamAbort) { @@ -251,7 +258,7 @@ virFDStreamCloseInt(virStreamPtr st, bool streamAbort) fdst-cb (fdst-events (VIR_STREAM_EVENT_READABLE | VIR_STREAM_EVENT_WRITABLE))) { -/* don't enter this function accidentaly from the callback again */ +/* don't enter this function accidentally from the callback again */ This hunk belongs in an earlier patch. if (fdst-abortCallbackCalled) { virMutexUnlock(fdst-lock); return 0; @@ -261,7 +268,7 @@ virFDStreamCloseInt(virStreamPtr st, bool streamAbort) fdst-abortCallbackDispatching = true; virMutexUnlock(fdst-lock); -/* call failure callback, poll does report nothing on closed fd */ +/* call failure callback, poll reports nothing on closed fd */ Same here. (fdst-cb)(st, VIR_STREAM_EVENT_ERROR, fdst-opaque); virMutexLock(fdst-lock); @@ -306,6 +313,14 @@ virFDStreamCloseInt(virStreamPtr st, bool streamAbort) st-privateData = NULL; +/* call the internal stream closing callback */ +if (fdst-icbCb) { +/* the mutex is not accessible anymore, as private data are null */ s/are/is/ +(fdst-icbCb)(st, fdst-icbOpaque); +if (fdst-icbFreeOpaque fdst-icbOpaque) Why are we refusing to call the icbFreeOpaque callback if icbOpaque is NULL? You should never make decisions based on an opaque pointer, because NULL might mean something to the callback. This should be: if (fdst-icbFreeOpaque) +(fdst-icbFreeOpaque)(fdst-icbOpaque); +} + if (fdst-dispatching) { fdst-closed = true; virMutexUnlock(fdst-lock); @@ -680,3 +695,23 @@ int virFDStreamCreateFile(virStreamPtr st, offset, length, oflags | O_CREAT, mode); } + +int virFDStreamSetInternalCloseCb(virStreamPtr st, + virFDStreamInternalCloseCb cb, + void *opaque, + virFDStreamInternalCloseCbFreeOpaque fcb) +{ +struct virFDStreamData *fdst = st-privateData; + +virMutexLock(fdst-lock); + +if (fdst-icbOpaque fdst-icbFreeOpaque) Same here. I'm not quite sure how you are planning on using this extra callback; I guess I need to continue on in the review; but assuming the use of this looks reasonable, I think you can fix the problems I pointed out and get my ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 6/7] util: Add helpers for safe domain console operations
On 02/06/2012 06:50 AM, Peter Krempa wrote: This patch adds a set of functions used in creating console streams for domains using PTYs and ensures mutually exclusive access to the PTYs. If mutualy exclusive access is not used, two clients may open the same s/mutualy/mutually/ console, which results in corruption on both clients as both of them race to read data from the PTY. Two approaches are used to ensure this: 1) Internal data structure holding open PTYs. This is used internally and enables the user to forcibly terminate another console connection eg. when somebody leaves the console open on another host. 2) UUCP style lock files: This uses UUCP lock files according to the FHS ( http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES ) to check if other programs (like minicom) are not using the pty device of the console. This feature is disabled by default and may be enabled using configure parameter --with-console-lock-files=/path/to/lock/file/directory or --with-console-lock-files=auto (which tries to infer the location from OS used (currently only linux). Should we also be modifying libvirt.spec.in to enable console lock files when building the RPM for Fedora? On usual linux systems, normal users may not write to the /var/lock directory containing the locks. This poses problems while in session mode. If the current user has no access to the lockfile directory, check for presence of the file is still done, but no lock file is created. This does NOT result in an error. --- configure.ac | 39 - po/POTFILES.in |1 + src/Makefile.am |6 +- src/conf/virconsole.c| 396 ++ src/conf/virconsole.h| 36 src/libvirt_private.syms |6 + 6 files changed, 475 insertions(+), 9 deletions(-) create mode 100644 src/conf/virconsole.c create mode 100644 src/conf/virconsole.h diff --git a/configure.ac b/configure.ac index 9fb7bfc..3254105 100644 --- a/configure.ac +++ b/configure.ac @@ -327,6 +327,12 @@ AC_ARG_WITH([remote], AC_HELP_STRING([--with-remote], [add remote driver support @:@default=yes@:@]),[],[with_remote=yes]) AC_ARG_WITH([libvirtd], AC_HELP_STRING([--with-libvirtd], [add libvirtd support @:@default=yes@:@]),[],[with_libvirtd=yes]) +AC_ARG_WITH([console-lock-files], + AC_HELP_STRING([--with-console-lock-files], + [location for UUCP style lock files for console PTYs + (use auto for default paths on some platforms) + @:@default=disabled@:@]), + [],[with_console_lock_files=disabled]) If I say ./configure --without-console-lock-files, then that defaults $with_console_lock_files=no. For consistency, I'd rather see the default be 'no', not 'disabled'; that way, you take advantage of autoconf's automatic --without-* support. Conversely, if I say ./configure --with-console-lock-files without an argument, autoconf defaults that to yes. I think you have to handle 'auto' and 'yes' in a similar manner, except the difference is that auto can gracefully degrade to 'no', while 'yes' errors out if we can't find a default location. dnl dnl in case someone want to build static binaries @@ -1197,6 +1203,22 @@ AM_CONDITIONAL([HAVE_AUDIT], [test $with_audit = yes]) AC_SUBST([AUDIT_CFLAGS]) AC_SUBST([AUDIT_LIBS]) +dnl UUCP style file locks for PTY consoles +if test $with_console_lock_files != disabled; then + if test $with_console_lock_files = auto; then +dnl Default locations for platforms +if test $with_linux = yes; then + with_console_lock_files=/var/lock +fi + fi + if test $with_console_lock_files = auto; then +AC_MSG_ERROR([You must specify path for the lock files on this platform]) + fi + AC_DEFINE_UNQUOTED([VIR_PTY_LOCK_FILE_PATH], $with_console_lock_files, + [path to directory containing UUCP pty lock files]) +fi So here, I would use: if test $with_console_lock_files != no; then if test $with_linux = yes; then with_console_lock_files=/var/lock elif test $with_console_lock_files = yes; then AC_MSG_ERROR([console lock files requested, but no path given]) elif test $with_console_lock_files = auto; then with_console_lock_files=no fi fi if test $with_console_lock_files != no; then AC_DEFINE_UNQUOTED([VIR_PTY_LOCK_FILE_PATH], ...) fi +AM_CONDITIONAL([VIR_PTY_LOCK_FILE_PATH], [test $with_console_lock_files != disabled]) and again, I'd compare to 'no', not 'disabled' + dnl SELinux AC_ARG_WITH([selinux], @@ -2751,14 +2773,15 @@ AC_MSG_NOTICE([ Alloc OOM: $enable_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([Debug: $enable_debug]) -AC_MSG_NOTICE([ Warnings:
Re: [libvirt] [PATCH v4 7/7] qemu: Add ability to abort existing console while creating new one
On 02/06/2012 06:50 AM, Peter Krempa wrote: This patch fixes console corruption, that happens if two concurrent sessions are opened for a single console on a domain. Result of this corruption was that each of the console streams recieved just a part s/recieved/received/ of the data written to the pipe so every console rendered unusable. New helper function for safe console handling is used to establish the console stream connection. This function ensures that no other libvirt client is using the console (with the ability to disconnect consoles of libvirt clients) and that no UUCP style lockfile is placed on the PTY device. * src/qemu/qemu_domain.h - add data structure to domain's private data dealing with console connections * src/qemu/qemu_domain.c: - allocate/free domain's console data structure * src/qemu/qemu_driver.c - use the new helper function for console handling --- src/qemu/qemu_domain.c |5 + src/qemu/qemu_domain.h |3 +++ src/qemu/qemu_driver.c | 21 - 3 files changed, 24 insertions(+), 5 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC: memory units=... [was: libvirt doesn't boot kVM, hangs, qemu on 100% CPU]
On 02/21/2012 03:46 PM, Igor Galić wrote: Hi folks, it's been adventurous. Yesterday night I've started debugging this particular issue of why my KVMs don't boot on Ubuntu 11.10. On IRC, we identified the culprit: uuid8cfcb7b0-10ee-7d08-9b64-9f39c154292a/uuid memory2048/memory currentMemory2048/currentMemory There ain't no way on earth you're going to boot a kernel in 2 megabytes of memory! I propose enhancing the XML; on output, libvirt should produce: memory units='k'2048/memory = 2048 * kibibyte the output unit must remain the same as it has always been, but the new attribute will make it easier for humans reading the XML to spot blunders like what spawned this thread. On input, the optional attribute is more useful - we can use it to provide a multiplier (of course, the result will be rounded up to k, and again rounded up to any higher granularity per the hypervisor): b = bytes k, KiB = kibibyte (1024) KB = kilobyte (1000) M, MiB = mebibyte (1024*1024) MB = megabyte (100) and so on for at least G and T (do we need P, E, Z, or Y? and I'm jealous if you have a machine with 1Y memory). Thoughts before I propose such a patch? -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V6] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On 02/16/2012 12:15 AM, Lai Jiangshan wrote: From: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com Changelog: - fixed typos. - fixed string scan routine. Signed-off-by: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- src/libvirt_private.syms |1 + src/nodeinfo.c | 92 ++ src/nodeinfo.h |3 + 3 files changed, 96 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0c22dec..6e99243 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -792,6 +792,7 @@ virNodeDeviceObjUnlock; # nodeinfo.h nodeCapsInitNUMA; +nodeGetCPUmap; nodeGetCPUStats; nodeGetCellsFreeMemory; nodeGetFreeMemory; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e0b66f7..fc1aaea 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -31,6 +31,7 @@ #include dirent.h #include sys/utsname.h #include sched.h +#include conf/domain_conf.h This is a header internal to libvirt, so it should use , not . And somehow, introducing this header pulled in enough other stuff to make the compiler complain about a later use of socket as a local variable name: nodeinfo.c: In function 'linuxNodeInfoCPUPopulate': nodeinfo.c:210:25: error: declaration of 'socket' shadows a global declaration [-Werror=shadow] cc1: all warnings being treated as errors #if HAVE_NUMACTL # define NUMA_VERSION1_COMPATIBILITY 1 @@ -569,6 +570,73 @@ int linuxNodeGetMemoryStats(FILE *meminfo, cleanup: return ret; } + +/* + * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set + * and max cpu is 7. The map file shows 0-4,6-7. This function parses + * it and returns cpumap. + */ +static const char * +linuxParseCPUmap(int *max_cpuid, const char *path) +{ +FILE *fp; +char *map = NULL; +char *str = NULL; +size_t size = 128, pos = 0; +int max_id, i; + +fp = fopen(path, r); +if (!fp) { +virReportSystemError(errno, _(cannot open %s), path); +goto error; +} + +if (VIR_ALLOC_N(str, size) 0) { +virReportOOMError(); +goto error; +} +for (;;) { +pos += fread(str + pos, 1, size - pos, fp); Rather than using an fread() loop, I think we can simplify things by using virFileReadAll(). +if (pos size) +break; + +size = size 1; +if (VIR_REALLOC_N(str, size) 0) { +virReportOOMError(); +goto error; +} +} +if (pos == 0) { +virReportSystemError(errno, _(cannot read from %s), path); +goto error; +} +str[pos - 1] = 0; + +if (VIR_ALLOC_N(map, VIR_DOMAIN_CPUMASK_LEN) 0) { +virReportOOMError(); +goto error; +} +if (virDomainCpuSetParse(str, 0, map, + VIR_DOMAIN_CPUMASK_LEN) 0) { +goto error; +} + +for (i = 0; i VIR_DOMAIN_CPUMASK_LEN; i++) { +if (map[i]) { +max_id = i; That's off by a factor of 8 - map[i] means that you have visited i*8 bits in cpumask. +} +} +*max_cpuid = max_id; + +VIR_FORCE_FCLOSE(fp); +return map; + +error: +VIR_FORCE_FCLOSE(fp); +VIR_FREE(str); +VIR_FREE(map); +return NULL; +} #endif int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { @@ -712,6 +780,30 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, #endif } +const char * +nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, + int *max_id ATTRIBUTE_UNUSED, + const char *mapname ATTRIBUTE_UNUSED) +{ +#ifdef __linux__ +char *path; +const char *cpumap; + +if (virAsprintf(path, CPU_SYS_PATH /%s, mapname) 0) { +virReportOOMError(); +return NULL; +} + +cpumap = linuxParseCPUmap(max_id, path); +VIR_FREE(path); +return cpumap; +#else + nodeReportError(VIR_ERR_NO_SUPPORT, %s, + _(node cpumap not implemented on this platform)); + return -1; Returning -1 as a const char * won't compile. You want NULL here. +#endif +} + #if HAVE_NUMACTL # if LIBNUMA_API_VERSION = 1 # define NUMA_MAX_N_CPUS 4096 diff --git a/src/nodeinfo.h b/src/nodeinfo.h index 4766152..852e19d 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -46,4 +46,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn, int maxCells); unsigned long long nodeGetFreeMemory(virConnectPtr conn); +const char *nodeGetCPUmap(virConnectPtr conn, + int *max_id, + const char *mapname); #endif /* __VIR_NODEINFO_H__*/ I'll see if I can fix these things myself - I'm anxious to get this pushed sooner rather than later. I'll continue my review, and if I polish the
Re: [libvirt] RFC: memory units=... [was: libvirt doesn't boot kVM, hangs, qemu on 100% CPU]
On Wed, Feb 22, 2012 at 04:42:59PM -0700, Eric Blake wrote: On 02/21/2012 03:46 PM, Igor Galić wrote: Hi folks, it's been adventurous. Yesterday night I've started debugging this particular issue of why my KVMs don't boot on Ubuntu 11.10. On IRC, we identified the culprit: uuid8cfcb7b0-10ee-7d08-9b64-9f39c154292a/uuid memory2048/memory currentMemory2048/currentMemory There ain't no way on earth you're going to boot a kernel in 2 megabytes of memory! I propose enhancing the XML; on output, libvirt should produce: memory units='k'2048/memory = 2048 * kibibyte the output unit must remain the same as it has always been, but the new attribute will make it easier for humans reading the XML to spot blunders like what spawned this thread. On input, the optional attribute is more useful - we can use it to provide a multiplier (of course, the result will be rounded up to k, and again rounded up to any higher granularity per the hypervisor): b = bytes k, KiB = kibibyte (1024) KB = kilobyte (1000) M, MiB = mebibyte (1024*1024) MB = megabyte (100) and so on for at least G and T (do we need P, E, Z, or Y? and I'm jealous if you have a machine with 1Y memory). Thoughts before I propose such a patch? I like the idea, maybe use units=KiB just to be completely clear? Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V6] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On 02/23/2012 08:10 AM, Eric Blake wrote: +if (virDomainCpuSetParse(str, 0, map, + VIR_DOMAIN_CPUMASK_LEN) 0) { +goto error; +} + +for (i = 0; i VIR_DOMAIN_CPUMASK_LEN; i++) { +if (map[i]) { +max_id = i; That's off by a factor of 8 - map[i] means that you have visited i*8 bits in cpumask. @map is not bitmask, virDomainCpuSetParse() filled it a char per a cpu. And the return of this function is also cpumap(byte per cpu). I will rework the patches soon after you comments them. Thank you very much. --lai. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: nicer error message on failed graceful destroy
On 02/22/2012 01:49 PM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=795656 mentions that a graceful destroy request can time out, meaning that the error message is user-visible and should be more appropriate than just internal error. * src/qemu/qemu_driver.c (qemuDomainDestroyFlags): Swap error type. --- src/qemu/qemu_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 717bdf1..c7ca881 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1792,7 +1792,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, */ if (flags VIR_DOMAIN_DESTROY_GRACEFUL) { if (qemuProcessKill(driver, vm, 0) 0) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +qemuReportError(VIR_ERR_OPERATION_FAILED, %s, _(failed to kill qemu process with SIGTERM)); goto cleanup; } ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] make memory use in XML easier to read
As suggested here: https://www.redhat.com/archives/libvir-list/2012-February/msg00954.html Eric Blake (3): xml: output memory unit for clarity xml: drop unenforced minimum memory limit from RNG xml: allow scaled memory on input docs/formatdomain.html.in | 39 - docs/schemas/domaincommon.rng | 27 +++- src/conf/domain_conf.c | 155 src/conf/domain_conf.h | 12 +- tests/define-dev-segfault |4 +- tests/domainschemadata/domain-lxc-simple.xml |2 +- tests/domainschemadata/portprofile.xml |2 +- .../qemu-simple-description-title.xml |4 +- tests/domainschemadata/timers.xml |4 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml |4 +- tests/domainsnapshotxml2xmlout/full_domain.xml |4 +- tests/domainsnapshotxml2xmlout/metadata.xml|4 +- tests/openvzutilstest.c|4 +- tests/qemuargv2xmltest.c |5 +- .../qemuxml2argv-balloon-device-auto.xml |4 +- .../qemuxml2argv-balloon-device.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-bios.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml |4 +- .../qemuxml2argv-blkiotune-device.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-blkiotune.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml |4 +- .../qemuxml2argv-boot-complex-bootindex.xml|4 +- .../qemuxml2argvdata/qemuxml2argv-boot-complex.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml |4 +- ...uxml2argv-boot-menu-disable-drive-bootindex.xml |4 +- .../qemuxml2argv-boot-menu-disable-drive.xml |4 +- .../qemuxml2argv-boot-menu-disable.xml |4 +- .../qemuxml2argv-boot-menu-enable.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-multi.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-boot-network.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml |4 +- .../qemuxml2argv-channel-guestfwd.xml |4 +- .../qemuxml2argv-channel-spicevmc-old.xml |2 +- .../qemuxml2argv-channel-spicevmc.xml |2 +- .../qemuxml2argv-channel-virtio-auto.xml |4 +- .../qemuxml2argv-channel-virtio.xml|4 +- .../qemuxml2argvdata/qemuxml2argv-clock-france.xml |4 +- .../qemuxml2argv-clock-localtime.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml |4 +- .../qemuxml2argv-clock-variable.xml|4 +- .../qemuxml2argv-console-compat-auto.xml |4 +- .../qemuxml2argv-console-compat-chardev.xml|4 +- .../qemuxml2argv-console-compat.xml|4 +- .../qemuxml2argv-console-virtio-many.xml |4 +- .../qemuxml2argv-console-virtio.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml |4 +- .../qemuxml2argv-cpu-exact2-nofallback.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml |4 +- .../qemuxml2argv-cpu-host-kvmclock.xml |4 +- .../qemuxml2argv-cpu-host-model-fallback.xml |4 +- .../qemuxml2argv-cpu-host-model-nofallback.xml |4 +- .../qemuxml2argv-cpu-host-model.xml|4 +- .../qemuxml2argv-cpu-host-passthrough.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml |4 +- .../qemuxml2argv-cpu-nofallback.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml |4 +- .../qemuxml2argv-cpu-qemu-host-passthrough.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml |4 +- .../qemuxml2argv-cpu-topology1.xml |4 +- .../qemuxml2argv-cpu-topology2.xml |4 +- .../qemuxml2argv-cpu-topology3.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-cputune.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-aio.xml |4 +- .../qemuxml2argv-disk-cdrom-empty.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml |4 +- .../qemuxml2argv-disk-copy_on_read.xml |2 +- .../qemuxml2argv-disk-drive-boot-cdrom.xml |4 +- .../qemuxml2argv-disk-drive-boot-disk.xml |4 +- .../qemuxml2argv-disk-drive-cache-directsync.xml |4 +- .../qemuxml2argv-disk-drive-cache-unsafe.xml |4 +- .../qemuxml2argv-disk-drive-cache-v1-none.xml |4 +-
[libvirt] [PATCH 2/3] xml: drop unenforced minimum memory limit from RNG
The test domain allows memory0/memory, but the RNG was stating that memory had to be at least 4096000 bytes. Hypervisors should enforce their own limits, rather than complicating the RNG. Meanwhile, some copy and paste had introduced some fishy constructs in various unit tests. * docs/schemas/domaincommon.rng (memoryKB, memoryKBElement): Drop limit that isn't enforced in code. * src/conf/domain_conf.c (virDomainDefParseXML): Require current = maximum. * tests/qemuxml2argvdata/*.xml: Fix offenders. --- docs/schemas/domaincommon.rng |2 -- src/conf/domain_conf.c |7 +++ .../qemuxml2argv-smartcard-controller.xml |2 +- .../qemuxml2argv-smartcard-host-certificates.xml |2 +- .../qemuxml2argv-smartcard-host.xml|2 +- ...qemuxml2argv-smartcard-passthrough-spicevmc.xml |2 +- .../qemuxml2argv-smartcard-passthrough-tcp.xml |2 +- .../qemuxml2argv-usb-controller.xml|2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml|2 +- .../qemuxml2argv-usb-ich9-companion.xml|2 +- .../qemuxml2argv-usb-ich9-ehci-addr.xml|2 +- .../qemuxml2argv-usb-piix3-controller.xml |2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml |2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml |2 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml |2 +- 15 files changed, 20 insertions(+), 15 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6b62f73..68e3fdc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3076,7 +3076,6 @@ define name=memoryKB data type=unsignedInt param name=pattern[0-9]+/param - param name=minInclusive4000/param /data /define !-- Memory as an element, with optional units attribute -- @@ -3088,7 +3087,6 @@ /optional data type='unsignedInt' param name='pattern'[0-9]+/param - param name='minInclusive'4000/param /data /define define name=domainName diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 07565bc..9dac731 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7275,6 +7275,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def-mem.cur_balloon) 0) def-mem.cur_balloon = def-mem.max_balloon; +if (def-mem.cur_balloon def-mem.max_balloon) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _(current memory '%luk' exceeds maximum '%luk'), + def-mem.cur_balloon, def-mem.max_balloon); +goto error; +} + node = virXPathNode(./memoryBacking/hugepages, ctxt); if (node) def-mem.hugepage_backed = 1; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml index b590066..601c308 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml @@ -2,7 +2,7 @@ nameQEMUGuest1/name uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid memory units='KiB'219136/memory - currentMemory units='KiB'219200/currentMemory + currentMemory units='KiB'219136/currentMemory vcpu1/vcpu os type arch='i686' machine='pc'hvm/type diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml index 3a7ed3d..0293df5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml @@ -2,7 +2,7 @@ nameQEMUGuest1/name uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid memory units='KiB'219136/memory - currentMemory units='KiB'219200/currentMemory + currentMemory units='KiB'219136/currentMemory vcpu1/vcpu os type arch='i686' machine='pc'hvm/type diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml index fbf0ddc..30582b7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml @@ -2,7 +2,7 @@ nameQEMUGuest1/name uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid memory units='KiB'219136/memory - currentMemory units='KiB'219200/currentMemory + currentMemory units='KiB'219136/currentMemory vcpu1/vcpu os type arch='i686' machine='pc'hvm/type diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml index 75fdd81..4c9ed0d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml @@ -2,7 +2,7 @@ nameQEMUGuest1/name
[libvirt] [PATCH 3/3] xml: allow scaled memory on input
Output is still in kibibytes, but input can now be in different scales for ease of typing. * src/conf/domain_conf.c (virDomainParseMemory): New helper. (virDomainDefParseXML): Use it when parsing. * docs/schemas/domaincommon.rng: Expand XML. * docs/formatdomain.html.in (elementsMemoryAllocation): Document scaling. --- docs/formatdomain.html.in | 39 ++--- docs/schemas/domaincommon.rng |4 +- src/conf/domain_conf.c| 129 ++-- 3 files changed, 142 insertions(+), 30 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5305f82..7a4a197 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -415,8 +415,8 @@ pre lt;domaingt; ... - lt;memorygt;524288lt;/memorygt; - lt;currentMemorygt;524288lt;/currentMemorygt; + lt;memory units='KiB'gt;524288lt;/memorygt; + lt;currentMemory units='KiB'gt;524288lt;/currentMemorygt; ... lt;/domaingt; /pre @@ -424,12 +424,27 @@ dl dtcodememory/code/dt ddThe maximum allocation of memory for the guest at boot time. -The units for this value are kilobytes (i.e. blocks of 1024 bytes)/dd +The units for this value are determined by the optional +atttribute codeunits/code, which defaults to KiB +(kibibytes, or blocks of 1024 bytes). Valid units are b or +bytes for bytes, KB for kilobytes (1,000), k or KiB +for kibibytes (1024), MB for megabytes (1,000,000), M or +MiB for mebibytes (1,048,576), GB for gigabytes +(1,000,000,000), G or GiB for gibibytes (1,073,741,824), +TB for terabytes (1,000,000,000,000), or T or TiB for +tebibytes (1,099,511,627,776). However, the value will be +rounded up to the nearest kibibyte by libvirt, and may be +further rounded to the granularity supported by the +hypervisor. As a sanity check, values less than 4000KiB are +not permitted. span class='since'codeunits/code since +0.9.11/span/dd dtcodecurrentMemory/code/dt ddThe actual allocation of memory for the guest. This value can be less than the maximum allocation, to allow for ballooning up the guests memory on the fly. If this is omitted, it defaults -to the same value as the codememory/code element/dd +to the same value as the codememory/code element. +The codeunits/code attribute behaves the same as +for codememory/code./dd /dl @@ -460,10 +475,10 @@ lt;domaingt; ... lt;memtunegt; -lt;hard_limitgt;1048576lt;/hard_limitgt; -lt;soft_limitgt;131072lt;/soft_limitgt; -lt;swap_hard_limitgt;2097152lt;/swap_hard_limitgt; -lt;min_guaranteegt;65536lt;/min_guaranteegt; +lt;hard_limit units='G'gt;1lt;/hard_limitgt; +lt;soft_limit units='M'gt;128lt;/soft_limitgt; +lt;swap_hard_limit units='G'gt;2lt;/swap_hard_limitgt; +lt;min_guarantee units='bytes'gt;67108864lt;/min_guaranteegt; lt;/memtunegt; ... lt;/domaingt; @@ -477,7 +492,13 @@ parameters are applied to the QEMU process as a whole. Thus, when counting them, one needs to add up guest RAM, guest video RAM, and some memory overhead of QEMU itself. The last piece is hard to -determine so one needs guess and try./dd +determine so one needs guess and try. For each tunable, it +is possible to designate which unit the number is in on +input, using the same values as +for codelt;memorygt;/code. For backwards +compatibility, output is always in +KiB. span class='since'codeunits/code +since 0.9.11/span/dd dtcodehard_limit/code/dt dd The optional codehard_limit/code element is the maximum memory the guest can use. The units for this value are kilobytes (i.e. blocks diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 68e3fdc..3d2e9f5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3082,7 +3082,9 @@ define name='memoryKBElement' optional attribute name='units' -valueKiB/value +data type='string' + param name='pattern'([bB]([yY][tT][eE][sS]?)?)|([kKmMgGtT]([iI]?[bB])?)/param +/data /attribute /optional data type='unsignedInt' diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9dac731..a869c70 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7141,6 +7141,96 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def, return 0; } + +/* Parse a memory element located at XPATH within CTXT, and store the + * result into MEM. If REQUIRED, then the value must exist; + * otherwise, the value is optional, but must result in at least 4000 + * blocks if present. Return 0 on success, -1 on failure after + * issuing error. */ +static int +virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt, +
[libvirt] [PATCH 1/3] xml: output memory unit for clarity
Make it obvious to 'dumpxml' readers what unit we are using, since our default of KiB (1024) differs from qemu's default of MiB. Tests were updated via: $ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/\(memory\|currentMemory\|hard_limit\|soft_limit\|min_guarantee\|swap_hard_limit\)/\1 units=''KiB'/ followed by a few fixes for the stragglers. * docs/schemas/domaincommon.rng (memoryKBElement): New define; use for memory elements. * src/conf/domain_conf.c (virDomainDefFormatInternal): Add units. * src/conf/domain_conf.h (_virDomainDef): Document unit used internally. * tests/*data/*.xml: Update all tests. * tests/*out/*.xml: Likewise. * tests/define-dev-segfault: Likewise. * tests/openvzutilstest.c (testReadNetworkConf): Likewise. * tests/qemuargv2xmltest.c (blankProblemElements): Likewise. --- Redundant portions of the patch omitted to fix mailing list limits. This email won't apply, but the elided portions should be obvious. docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 21 src/conf/domain_conf.h | 12 tests/define-dev-segfault |4 +- tests/domainschemadata/domain-lxc-simple.xml |2 +- tests/domainschemadata/portprofile.xml |2 +- .../qemu-simple-description-title.xml |4 +- tests/domainschemadata/timers.xml |4 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml |4 +- tests/domainsnapshotxml2xmlout/full_domain.xml |4 +- tests/domainsnapshotxml2xmlout/metadata.xml|4 +- tests/openvzutilstest.c|4 +- tests/qemuargv2xmltest.c |5 ++- .../qemuxml2argv-balloon-device-auto.xml |4 +- .../qemuxml2argv-balloon-device.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-bios.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml |4 +- .../qemuxml2argv-blkiotune-device.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-blkiotune.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml |4 +- .../qemuxml2argv-boot-complex-bootindex.xml|4 +- .../qemuxml2argvdata/qemuxml2argv-boot-complex.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml |4 +- ...uxml2argv-boot-menu-disable-drive-bootindex.xml |4 +- .../qemuxml2argv-boot-menu-disable-drive.xml |4 +- .../qemuxml2argv-boot-menu-disable.xml |4 +- .../qemuxml2argv-boot-menu-enable.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-multi.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-boot-network.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml |4 +- .../qemuxml2argv-channel-guestfwd.xml |4 +- .../qemuxml2argv-channel-spicevmc-old.xml |2 +- .../qemuxml2argv-channel-spicevmc.xml |2 +- .../qemuxml2argv-channel-virtio-auto.xml |4 +- .../qemuxml2argv-channel-virtio.xml|4 +- .../qemuxml2argvdata/qemuxml2argv-clock-france.xml |4 +- .../qemuxml2argv-clock-localtime.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml |4 +- .../qemuxml2argv-clock-variable.xml|4 +- .../qemuxml2argv-console-compat-auto.xml |4 +- .../qemuxml2argv-console-compat-chardev.xml|4 +- .../qemuxml2argv-console-compat.xml|4 +- .../qemuxml2argv-console-virtio-many.xml |4 +- .../qemuxml2argv-console-virtio.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml |4 +- .../qemuxml2argv-cpu-exact2-nofallback.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml |4 +- .../qemuxml2argv-cpu-host-kvmclock.xml |4 +- .../qemuxml2argv-cpu-host-model-fallback.xml |4 +- .../qemuxml2argv-cpu-host-model-nofallback.xml |4 +- .../qemuxml2argv-cpu-host-model.xml|4 +- .../qemuxml2argv-cpu-host-passthrough.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml |4 +- .../qemuxml2argv-cpu-nofallback.xml|4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml |4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml |4 +- .../qemuxml2argv-cpu-qemu-host-passthrough.xml |4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml |4 +- .../qemuxml2argv-cpu-topology1.xml |4 +- .../qemuxml2argv-cpu-topology2.xml |4 +-
[libvirt] Cluster_size parameter issue on qcow2 image format
I have been working o the qcow2 image format , I theory regarding cluster size it is written that as the size of cluster increase performance should increase. But something surprising happen The performance is degrading as the size of cluster increased from 64K to 1M ( during expansion of qcow2 image) can any one tell why ? DISCLAIMER: --- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . - libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Forbid migration with cache != none
On 02/22/2012 02:57 PM, Jiri Denemark wrote: how about disk write through flag here? This flag should also imply cache=none. i.e, VIR_DOMAIN_DISK_CACHE_WRITETHRU AFAIK, only VIR_DOMAIN_DISK_CACHE_DISABLE translates into cache=none unless qemu binary is old enough to only support cache=on|off VIR_DOMAIN_DISK_CACHE_DIRECTSYNC too. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list