December 2014 - Devel - libvirt List Archives

[libvirt] [PATCH 00/11] Fix vm usage after ExitMonitor
by Ján Tomko 17 Dec '14

17 Dec '14

Check if the domain died while we were in the monitor in most of the places (I didn't look at the Snapshot and Blockjob APIs yet). In some places, we've been accessing the data after it could've been freed by qemuProcessStop (if the monitor API itself returned success, but the domain was cleaned up because somehow qemuProcessStop got the vm lock before the ExitMonitor call). A more likely crash happens if qemu crashes in one of the APIs with multiple Monitor calls, like the DetachDevice APIs and AttachCharDevice. Also, some occurences of writing the status XML for a dead domain are fixed. https://bugzilla.redhat.com/show_bug.cgi?id=1161024 Ján Tomko (11): Introduce qemuDomainObjExitMonitorAlive Mark the domain as active in qemuhotplugtest Fix vm usage after ExitMonitor in UpdateDeviceList Fix vm usage after ExitMonitor on device removal Fix error message on redirdev caps detection Fix vm usage after ExitMonitor on AttachDevice Fix vm usage after ExitMonitor in DetachDevice Fix monitor usage in qemuDomainHotplugVcpus Fix vm usage after ExitMonitor in qemu driver Fix vm usage after ExitMonitor in qemu migration Fix vm usage after ExitMonitor in qemu process src/qemu/qemu_command.c | 6 +- src/qemu/qemu_domain.c | 20 +++++- src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 49 ++++++++++---- src/qemu/qemu_hotplug.c | 169 +++++++++++++++++++++++++++++++--------------- src/qemu/qemu_migration.c | 41 +++++++---- src/qemu/qemu_process.c | 65 ++++++++++++------ tests/qemuhotplugtest.c | 4 ++ 8 files changed, 251 insertions(+), 106 deletions(-) -- 2.0.4

3 25

[libvirt] [PATCH] conf: fix cannot start a guest have a shareable network iscsi hostdev
by Luyao Huang 17 Dec '14

17 Dec '14

https://bugzilla.redhat.com/show_bug.cgi?id=1174569 We should do nothing for the shareable network iscsi hostdev in qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for a network iscsi hostdev is not valid, so just ignore it. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- src/qemu/qemu_conf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8d3818e..9539231 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1158,7 +1158,8 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver, if (!hostdev->shareable || !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) return 0; if (!(key = qemuGetSharedHostdevKey(hostdev))) @@ -1261,7 +1262,8 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver, if (!hostdev->shareable || !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) return 0; if (!(key = qemuGetSharedHostdevKey(hostdev))) -- 1.8.3.1

3 4

[libvirt] [PATCHv2] logical: Add "--type snapshot" to lvcreate command
by John Ferlan 17 Dec '14

17 Dec '14

A recent lvm change has resulted in a change for the "default" type of logical volume created when the "--virtualsize" or "--V" is supplied on the command line (e.g. when the allocation and capacity values of a to be created volume differ). It seems that at the very least the following change adjusts the default type: https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=e0164f21 and the following may also have some impact. https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=87fc3b71 When using the virsh vol-create-as or vol-create xmlfile commands, the result is that libvirt will now create a "thin logical volume" and a "thin logical volume pool" rather than just a "thin snapshot logical volume". For example the following sequence: # lvcreate --name test -L 2M -V 5M lvm_test Rounding up size to full physical extent 4.00 MiB Rounding up size to full physical extent 8.00 MiB Logical volume "test" created. # lvs lvm_test LV VG Attr LSize Pool Origin Data% Meta% Move Log Cpy%Sync Convert lvol1 lvm_test twi-a-tz-- 4.00m 0.00 0.98 test lvm_test Vwi-a-tz-- 8.00m lvol1 0.00 compared to the former code which had the following: LV VG Attr LSize Pool Origin Data% Move Log Cpy%Sync Convert test LVM_Test swi-a-s--- 4.00m [test_vorigin] 0.00 Since libvirt doesn't know how to parse the thin logical volume and pool, it will fail to find the newly created volume and pool even though it exists in the volume group. It cannot find since the command used to find/parse returns a thin volume 'test' with no associated device, for example the output is: lvol1##UgUwkp-fTFP-C0rc-ufue-xrYh-dkPr-FGPFPx#lvol1_tdata(0)#thin-pool#1#4194304#4194304#4194304#twi-a-tz-- test##NcaIoH-4YWJ-QKu3-sJc3-EOcS-goff-cThLIL##thin#0#8388608#4194304#8388608#Vwi-a-tz-- as compared to the former which had the following: test#[test_vorigin]#Dt5Of3-4WE6-buvw-CWJ4-XOiz-ywOU-YULYw6#/dev/sda3(1300)#linear#1#4194304#4194304#4194304#swi-a-s--- While it's possible to generate code to handle the new thin lv and pool, this patch will add a "--type snapshot" onto the lvcreate command libvirt uses in order to "for now" be able to continue to utilize the thin snapshots Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- v1: http://www.redhat.com/archives/libvir-list/2014-December/msg00705.html The first version replaces thin snapshots with thin volumes and pools. Although it got an ACK, Jan Tomko points out rightfully so that perhaps we need more time to vet a change such as this - since it could be considered a few feature. So since using --type snapshot restores former functionality that should suffice until we can make sure we understand more about the differences between thin snapshots and thin volumes. src/storage/storage_backend_logical.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4959985..8aa68a6 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -758,6 +758,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.allocation ? vol->target.allocation : 1, 1024)); + virCommandAddArgList(cmd, "--type", "snapshot", NULL); virCommandAddArg(cmd, "--virtualsize"); vol->target.sparse = true; } -- 1.9.3

2 1

[libvirt] [PATCH] conf: move the check for secondary consoles of targetType serial
by Erik Skultety 17 Dec '14

17 Dec '14

For historical reasons, only the first <console> element might be of targetType serial, but we checked for other consoles of targetType serial in our post-parse callback if and only if we knew the first console was serial, otherwise the check was skipped. This patch moves the check one level up, so first the check for secondary console of type serial is performed and then the rest of operations continue unchanged. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1170092 --- src/conf/domain_conf.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ec45b8c..dc0b167 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3023,19 +3023,22 @@ virDomainDefPostParseInternal(virDomainDefPtr def, * We then fill def->consoles[0] with a stub just so we get sequencing * correct for consoles > 0 */ + + /* Only the first console (if there are any) can be of type serial, + * verify that no other console is of type serial + */ + for (i = 1; i < def->nconsoles; i++) { + virDomainChrDefPtr cons = def->consoles[i]; + + if (cons->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only the first console can be a serial port")); + return -1; + } + } if (def->nconsoles > 0 && STREQ(def->os.type, "hvm") && (def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL || def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)) { - /* First verify that only the first console is of type serial */ - for (i = 1; i < def->nconsoles; i++) { - virDomainChrDefPtr cons = def->consoles[i]; - - if (cons->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only the first console can be a serial port")); - return -1; - } - } /* If there isn't a corresponding serial port: * - create one and set, the console to be an alias for it -- 1.9.3

2 3

[libvirt] [PATCH] avoid using deprecated udev logging functions
by Daniel P. Berrange 17 Dec '14

17 Dec '14

In systemd >= 218, the udev_set_log_fn method has been marked deprecated and turned into a no-op. Nothing in the udev client library will print to stderr by default anymore, so we can just stop installing a logging hook for new enough udev. --- m4/virt-udev.m4 | 7 +++++++ src/node_device/node_device_udev.c | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/m4/virt-udev.m4 b/m4/virt-udev.m4 index 55673bf..0d785c4 100644 --- a/m4/virt-udev.m4 +++ b/m4/virt-udev.m4 @@ -24,6 +24,13 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[ if test "$with_udev" = "yes" && test "$with_pciaccess" != "yes" ; then AC_MSG_ERROR([You must install the pciaccess module to build with udev]) fi + + if test "$with_udev" = "yes" ; then + PKG_CHECK_EXISTS(libudev >= 218, [with_udev_logging=no], [with_udev_logging=yes]) + if test "$with_udev_logging" = "yes" ; then + AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1, [whether libudev logging can be used]) + fi + fi ]) AC_DEFUN([LIBVIRT_RESULT_UDEV],[ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 343ef85..1a35c07 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -340,7 +340,7 @@ static int udevGenerateDeviceName(struct udev_device *device, return ret; } - +#if HAVE_UDEV_LOGGING typedef void (*udevLogFunctionPtr)(struct udev *udev, int priority, const char *file, @@ -373,6 +373,7 @@ udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED, VIR_FREE(format); } +#endif static int udevTranslatePCIIds(unsigned int vendor, @@ -1759,8 +1760,10 @@ static int nodeStateInitialize(bool privileged, * its return value. */ udev = udev_new(); +#if HAVE_UDEV_LOGGING /* cast to get rid of missing-format-attribute warning */ udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction); +#endif priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); if (priv->udev_monitor == NULL) { -- 2.1.0

3 4

[libvirt] [PATCH V2 0/5] support sending sysrq key
by Chunyan Liu 16 Dec '14

16 Dec '14

xend/libxl support sending sysrq key to guest kernel but not support sending any key sequence as virDomainSendKey is expected to do. To add equivalant sysrq functionality in libvirt for xen/libxl, add a new virDomainSendSysrq API and add related codes to virsh, remote driver, xen/libxl driver. Changes: * add new API virDomainSendSysrq instead of hacking virDomainSendKey. * add related changes according to new API, including libvirt API, remote protocol, virsh and libxl/xen driver. v1 is here: https://www.redhat.com/archives/libvir-list/2014-December/msg00480.html Chunyan Liu (5): Add public API virDomainSendSysrq implement remote protocol for domainSendSysrq virsh: add 'sysrq' command libxl: implement .domainSendSysrq method xen: add .domainSendSysrq method include/libvirt/libvirt-domain.h | 3 +++ src/driver-hypervisor.h | 4 +++ src/libvirt-domain.c | 38 ++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/libxl/libxl_driver.c | 23 +++++++++++++++++ src/remote/remote_driver.c | 2 +- src/remote/remote_protocol.x | 13 +++++++++- src/xen/xen_driver.c | 19 ++++++++++++++ src/xen/xend_internal.c | 20 +++++++++++++++ src/xen/xend_internal.h | 2 ++ tools/virsh-domain.c | 54 ++++++++++++++++++++++++++++++++++++++++ 11 files changed, 177 insertions(+), 2 deletions(-) -- 1.8.4.5

7 21

[libvirt] virDomainGetBlockInfo meanings
by Eric Blake 16 Dec '14

16 Dec '14

I'm really struggling with how to interpret the virDomainBlockInfo struct, and think that we may want to consider the current behavior buggy and offer improved behavior going forward. For starters, even though the stats reported by virDomainListGetStats are _supposed_ to mirror the older API of virDomainGetBlockInfo, they currently do not (the newer API reads values from qemu, the older API reads values from stat) - in order to make the two consistent, I need to share code - but in order to share code, I need to know WHICH values make the most sense. Here's the current documentation: /** virDomainBlockInfo: * * This struct provides information about the size of a block device * backing store * * Examples: * * - Fully allocated raw file in filesystem: * * capacity, allocation, physical: All the same * * - Sparse raw file in filesystem: * * capacity: logical size of the file * * allocation, physical: number of blocks allocated to file * * - qcow2 file in filesystem * * capacity: logical size from qcow2 header * * allocation, physical: logical size of the file / * highest qcow extent (identical) * * - qcow2 file in a block device * * capacity: logical size from qcow2 header * * allocation: highest qcow extent written for an active domain * * physical: size of the block device container */ typedef struct _virDomainBlockInfo virDomainBlockInfo; typedef virDomainBlockInfo *virDomainBlockInfoPtr; struct _virDomainBlockInfo { unsigned long long capacity; /* logical size in bytes of the block device backing image */ unsigned long long allocation; /* highest allocated extent in bytes of the block device backing image */ unsigned long long physical; /* physical size in bytes of the container of the backing image */ }; I interpret this to mean that 'capacity' is the size that the guest sees, that 'allocation' is how much host space is required to represent that to the guest, and 'physical' is the size of the container on the host. So there are at least two things that strike me as incorrect or confusing in that documentation. First, our usage of 'physical' in the examples _changes_ depending on whether the file is raw or a block device. I think that 'physical' should represent the size of a file that 'ls' would show, no matter what, while 'allocation' is what 'du' would show. That is, for the 'sparse raw file in filesystem', I think we want 'capacity == physical == size guest would see', and only 'allocation' would be smaller; our example of 'physical == allocation' for a sparse file sounds wrong, because 'physical' should be the size of the container (the host file) rather than the amount of host storage used. Second, when this struct was first introduced, it predated the ability of the kernel to punch holes in a file. But now qemu can do a very good job of reclaiming space in a qcow2 file by punching holes and making the file sparse. So I think that 'qcow2 file in filesystem' should allow for 'allocation < physical' - again, with the notion that 'physical' is the size of the qcow2 file in the host that 'ls' would show, and 'allocation' is the storage occupied on the host as 'du' would show. This would make things more consistent with the 'qcow2 file in a block device', where 'physical' is the size of the device (well, 'ls' doesn't show that, so you have to open/lseek the device to find it) and where 'allocation' is the amount of the device used (here, relying on qemu to tell us the highest extent written so far, since block devices don't have holes). Oh, and qemu has a limitation - it only reports highest extent written when it actually writes to an image; if a file is opened read-only (such as a backing file), qemu reports 0 as the highest extent written, even though that cannot be true (for a qcow2 file to exist, at least the first cluster must have been previously written with metadata) - so it would be nice if future qemu could be patched to determine the highest extent when opening an image, rather than on first write to the image. Also, it is annoying that qemu splits its reporting: 'capacity' is reported via 'query-block' (the "actual-size" value); this command also gives what appears to be a great 'physical' value for files (the "actual-size" parameter) although that size is always 0 for block devices; meanwhile, the highest extent written ('allocation' for block devices) is reported via 'query-blockstats' (the "wr_highest_offset" value). Can anyone give a reason why changing semantics of 'physical' and 'allocation' as mentioned above would negatively break backwards compatibility, rather than being considered a bug fix? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2 2

[libvirt] [PATCH] Buffer size too small when reading sysinfo on S390
by Boris Fiuczynski 16 Dec '14

16 Dec '14

On a system with 160 CPUs the /proc/cpuinfo size grows beyond the currently set limit of 10KB causing an internal error. This patch increases the buffer size for S390 to 64KB. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com> --- src/util/virsysinfo.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 1bb6392..613bab9 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -377,6 +377,8 @@ virSysinfoRead(void) # pragma GCC diagnostic ignored "-Wlogical-op" # endif +# define S390_CPUINFO_FILE_LEN (64*1024) /* 64KB limit for /proc/cpuinfo file */ + static char * virSysinfoParseDelimited(const char *base, const char *name, char **value, char delim1, char delim2) @@ -471,7 +473,7 @@ virSysinfoRead(void) goto no_memory; /* Gather info from /proc/cpuinfo */ - if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) { + if (virFileReadAll(CPUINFO, S390_CPUINFO_FILE_LEN, &outbuf) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to open %s"), CPUINFO); return NULL; -- 1.9.3

3 3

[libvirt] How to use blockcopy in libvirt-java
by Star Guo 16 Dec '14

16 Dec '14

Hi, ALL, I just look for the class and method to use blockcopy in libvirt-java on this page: http://libvirt.org/sources/java/javadoc/ However, I can not find out. Libvirt-java Support blockcopy ? Thanks. Best Regards, Star Guo

2 1

[libvirt] [RFC] The Quest for Domain-IP Mapping Continues...
by Nehal J Wani 16 Dec '14

16 Dec '14

If you remember correctly, about 1.2 years ago, we had... https://www.redhat.com/archives/libvir-list/2013-September/msg00351.html In short, the patch proposed a generalized mechanism for finding out IP addresses, with flags for choosing method(s) (SNOOP|LEASE|AGENT). But since not all methods were supported (only qemu-ga worked), the API was deferred. After exploring a bit, it seems that in all, we can have: (i) Leases Method (ii) Qemu Guest Agent (iiI) DHCP Snooping (exists, but isn't publicly exposed?) (iv) ARP Snooping (we can ask the virtual network interfaces to share their arp tables with the public?) Now that we have the dhcp-leases API fully furnished, why not visit the qemu-guest-agent again? OP1. Should we have separate APIs + virsh commands for each one of these and then combine them together under virDomainInterfaceAddresses? - If each one has to be a different API and then combined later, then what should be the corresponding name for the virsh command for exposing the querying guest agent API? OP2. Implement virDomainInterfaceAddresses API with two flags, one for dhcp-leases and another for qemu-ga and keep adding others whenever they are completed? I would prefer OP1. What do you guys think? -- Nehal J Wani

3 5