Re: [libvirt] [PATCH v4 2/3] lxc: Modify/add some debug messages
On Thu, Feb 12, 2015 at 03:48:11PM -0500, John Ferlan wrote: Modify the VIR_DEBUG message in virLXCProcessCleanup to make it clearer about the path. Also add some more VIR_DEBUG messages in virLXCProcessStart in order to help debug error flow. Signed-off-by: John Ferlan jfer...@redhat.com --- src/lxc/lxc_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) ACK 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
Re: [libvirt] [PATCH v4 1/3] lxc: Move console checks in LXCProcessStart
On Thu, Feb 12, 2015 at 03:48:10PM -0500, John Ferlan wrote: From: Luyao Huang lhu...@redhat.com https://bugzilla.redhat.com/show_bug.cgi?id=1176503 Move the two console checks - one for zero nconsoles present and the other for an invalid console type to earlier in the processing rather than getting after performing some setup that has to be undone for what amounts to an invalid configuration. This resolves the above bug since it's not not possible to have changed the security labels when we cause the configuration check failure. Signed-off-by: John Ferlan jfer...@redhat.com ACK 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
Re: [libvirt] [PATCH v4 3/3] lxc: Fix container cleanup for LXCProcessStart
On Thu, Feb 12, 2015 at 03:48:12PM -0500, John Ferlan wrote: From: Luyao Huang lhu...@redhat.com Jumping to the cleanup label prior to starting the container failed to properly clean everything up that is handled by the virLXCProcessCleanup which is called if virLXCProcessStop is called on failure after the container properly starts. Most importantly is prior to this patch none of the stop/release hooks, host device reattachment, and network cleanup (that is reverse of virLXCProcessSetupInterfaces). Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: John Ferlan jfer...@redhat.com --- src/lxc/lxc_process.c | 78 ++- 1 file changed, 33 insertions(+), 45 deletions(-) ACK 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
Re: [libvirt] [PATCH 0/3] fsfreeze and fsthaw API testing
On 04/02/15 17:16, Jincheng Miao wrote: Added guest agent conf to domain xml in order to test API fsfreeze and fsthaw. Jincheng Miao (3): add guest agent configuration to domain xml Here is a conflict with your former commit 9d0feb81224075daa82, please check it again. Add domain fsfreeze and fsthaw API testing Please remove extra spaces from above patch Add fsfreeze fsthaw test case to linux_domain cases/linux_domain.conf| 18 + repos/domain/domain_fsfreeze.py| 76 repos/domain/domain_fsthaw.py | 49 + .../domain/xmls/kvm_linux_guest_install_cdrom.xml |4 + repos/domain/xmls/kvm_linux_guest_install_net.xml | 20 +++-- 5 files changed, 159 insertions(+), 8 deletions(-) create mode 100644 repos/domain/domain_fsfreeze.py create mode 100644 repos/domain/domain_fsthaw.py I try to run it in fedora 21 with below packages, qemu did not freeze the given mountpoint path. If I used a wrong version, what versions did you test during coding this API? [root@hp-dl320eg8-07 libvirt-test-API]# rpm -q libvirt-python qemu libvirt-python-1.2.7-2.fc21.x86_64 qemu-2.1.3-2.fc21.x86_64 Scripts ouput: ++ [root@hp-dl320eg8-07 libvirt-test-API]# python libvirt-test-api -c ff.conf -l 0 Checking Testing Environment... Linux hp-dl320eg8-07.qe.lab.eng.nay.redhat.com 3.16.1-301.fc21.x86_64 #1 SMP Mon Aug 25 13:06:39 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Virsh command line tool of libvirt: 1.2.7 libvirtd (libvirt) 1.2.7 Default URI: qemu:///system QEMU emulator version 2.1.3 (qemu-2.1.3-2.fc21), Copyright (c) 2003-2008 Fabrice Bellard Start Testing: Case Count: 4 Log File: log/20150213164835/libvirt_test001 domain:domain_fsfreeze 16:48:36|INFO |freeze 2 fs 16:48:36|INFO |Check frozen fs num: pass Result: OK domain:domain_fsthaw 16:48:36|INFO |fsThaw 2 fs Result: OK domain:domain_fsfreeze 16:48:36|INFO |freeze 2 fs 16:48:36|ERROR |Check frozen fs num: failed Result: FAIL domain:domain_fsthaw 16:48:36|INFO |fsThaw 2 fs Result: OK Summary: Total:4 [Pass:3 Fail:1 Skip:0] +++ For the rest, it's OK for me. BR, Jianwei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] QEMU capabilities vs machine types
On 12.02.2015 20:25, Eduardo Habkost wrote: On Wed, Feb 11, 2015 at 05:09:01PM +0100, Michal Privoznik wrote: On 11.02.2015 16:47, Daniel P. Berrange wrote: On Wed, Feb 11, 2015 at 04:31:53PM +0100, Michal Privoznik wrote: There are two reasons why we query check the supported capabilities from QEMU 1. There are multiple possible CLI args for the same feature and we need to choose the best one to use 2. The feature is not supported and we want to give the caller a better error message than they'd get from QEMU I'm unclear from the bug which scenario applies here. If it is scenario 2 though, I'd just mark it as CANTFIX or WONTFIX, as no matter what we do the user would get an error. It is not worth making our capability matrix a factor of 10+ bigger just to get a better error message. If it is scenario 1, I think the burden is on QEMU to solve. The memory-backend-{file,ram} CLI flags shouldn't be tied to guest machine types, as they are backend config setup options that should not impact guest ABI. It's somewhere in between 1 and 2. Back in RHEL-7.0 days libvirt would have created a guest with: -numa node,...,mem=1337 But if qemu reports it support memory-backend-ram, libvirt tries to use it: -object memory-backend-ram,id=ram-node0,size=1337M,... \ -numa node,...,memdev=ram-node0 This breaks migration to newer qemu which is in RHEL-7.1. If qemu would report the correct value, we can generate the correct command line and migration succeeds. However, our fault is, we are not asking the correct question anyway. I understand that RHEL-7.1 QEMU is not providing enough data for libvirt to detect this before it is too late. What I am missing here is: why wasn't commit f309db1f4d51009bad0d32e12efc75530b66836b enough to fix this specific case? The numa pinning can be expressed in libvirt in this way: numatune memory mode='strict' nodeset='0-7'/ memnode cellid='0' mode='preferred' nodeset='3'/ memnode cellid='2' mode='strict' nodeset='1-2,5,7'/ /numatune This tells, to pin guest #0 onto host #3, guest #2 onto host #1-2,5, or 7. For the rest of guest numa nodes, they are placed onto host #0-7. As long as there explicit guest guest numa node pinning onto host nodes (the memnode/ element), memory-object-ram is required. However, if numatune/ has only one child memory/ we still can guarantee the requested configuration in CGroups and don't necessarily need memory-object-ram. My patch, you've referred to, was incomplete in this case. Moreover, it was buggy, it allowed combining use of bare -numa and memory-object-ram at the same time (which is not allowed). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] attach-interface: Learn net type='direct'
On 02/12/2015 04:50 PM, John Ferlan wrote: On 02/09/2015 10:20 AM, Michal Privoznik wrote: v2 Michal Privoznik (4): libvirt_private.syms: Expose virDomainNetTypeFromString virsh attach-interface: Use enum instead of arbitrary integers virsh attach-interface: Use virDomainNetType{From,To}String() virsh attach-interface: Allow macvtap hotplug src/libvirt_private.syms | 1 + tools/virsh-domain.c | 36 ++-- tools/virsh.pod | 14 -- 3 files changed, 35 insertions(+), 16 deletions(-) ACK series... I had thought that during V1 of this patch series there had been enough sentiment about not pulling in private parts of libvirt internals that this V2 (posted 2 minutes before the last comment in V1, so very literally crossing in the inter-tubes) would be discarded in favor of something that only used public APIs - the last word on the subject was this: https://www.redhat.com/archives/libvir-list/2015-February/msg00284.html (Yes, I know there are other places where it's already done, but it seems like a better idea to clean those up than to compound the pollution.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemuCaps: Disable memdev for rhel6.5.0 machine type
On Thu, Feb 12, 2015 at 04:09:40PM +0100, Michal Privoznik wrote: Well, after [1] qemu doesn't understand '-object memory-backend-ram' nor '-object memory-backend-file'. Make sure we remove that capabilities from our internal list temporarily, so the qemu command line is constructed in correct way. 1: https://bugzilla.redhat.com/show_bug.cgi?id=1170093 Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_capabilities.c | 10 --- .../qemuxml2argv-numatune-memnode-rhel650.args | 7 + .../qemuxml2argv-numatune-memnode-rhel650.xml | 31 ++ tests/qemuxml2argvtest.c | 4 +++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 233449b..940f070 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3510,6 +3510,11 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps) return sb.st_ctime == qemuCaps-ctime; } +static virQEMUCapsFlags virQEMUCapsMachineRHEL650Filter[] = { +/* For some reason, rhel6.5.0 machine type doesn't understand memdev. */ +QEMU_CAPS_OBJECT_MEMORY_RAM, +QEMU_CAPS_OBJECT_MEMORY_FILE, +}; struct virQEMUCapsMachineTypeFilter { const char *machineType; @@ -3518,9 +3523,8 @@ struct virQEMUCapsMachineTypeFilter { }; static const struct virQEMUCapsMachineTypeFilter virQEMUCapsMachineFilter[] = { -/* { blah, virQEMUCapsMachineBLAHFilter, - ARRAY_CARDINALITY(virQEMUCapsMachineBLAHFilter) }, */ -{ , NULL, 0 }, +{ rhel6.5.0, virQEMUCapsMachineRHEL650Filter, +ARRAY_CARDINALITY(virQEMUCapsMachineRHEL650Filter)}, }; FWIW, I'd consider this to be something that RHEL downstream RPMs should carry, not for upstream. 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
Re: [libvirt] [PATCH v8 1/4] domifaddr: Implement the public APIs
On 01/25/2015 01:38 PM, Nehal J Wani wrote: Define helper function virDomainInterfaceFree, which allows the upper layer application to free the domain interface object conveniently. The API is going to provide multiple methods by flags, e.g. * Query guest agent * Parse DHCP lease file include/libvirt/libvirt-domain.h * Define virDomainInterfaceAddresses, virDomainInterfaceFree * Define structs virDomainInterface, virDomainIPAddress src/driver-hypervisor.h: * Define domainInterfaceAddresses src/libvirt-domain.c: * Implement virDomainInterfaceAddresses * Implement virDomainInterfaceFree src/libvirt_public.syms: * Export the new symbols Signed-off-by: Nehal J Wani nehaljw.k...@gmail.com --- include/libvirt/libvirt-domain.h | 27 src/driver-hypervisor.h | 5 ++ src/libvirt-domain.c | 129 +++ src/libvirt_public.syms | 6 ++ 4 files changed, 167 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4dbd7f5..1f832d0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3682,5 +3682,32 @@ typedef struct _virTypedParameter virMemoryParameter; */ typedef virMemoryParameter *virMemoryParameterPtr; +typedef enum { +VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE = (1 0), /* Parse DHCP lease file */ +VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT = (1 1), /* Query qemu guest agent */ +} virDomainInterfaceAddressesFlags; + +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress; +typedef virDomainIPAddress *virDomainIPAddressPtr; +struct _virDomainInterfaceIPAddress { +int type;/* virIPAddrType */ +char *addr; /* IP address */ +unsigned int prefix; /* IP address prefix */ +}; + +typedef struct _virDomainInterface virDomainInterface; +typedef virDomainInterface *virDomainInterfacePtr; +struct _virDomainInterface { +char *name; /* interface name */ +char *hwaddr; /* hardware address */ +unsigned int naddrs;/* number of items in @addrs */ +virDomainIPAddressPtr addrs;/* array of IP addresses */ +}; + +int virDomainInterfaceAddresses(virDomainPtr dom, +virDomainInterfacePtr **ifaces, +unsigned int flags); + +void virDomainInterfaceFree(virDomainInterfacePtr iface); While I haven't followed this from the first RFC or taken the time to look at all 8 patches, I'll assume this set of data has been agreed upon as the relatively important set of IP Address and Network Interface data. Looking at the .0 comments it seems the desire was for some more flexibility to handle future possible issues - I guess my comment there is - IPv4 isn't changing much and getting IPv6 adoption as the norm seems to have been an uphill religious battle for quite a few years - so as they say - change isn't very frequent thus perhaps this is a safe set of data to collect/display #endif /* __VIR_LIBVIRT_DOMAIN_H__ */ ...snip... diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 492e90a..4149332 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11249,3 +11249,132 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) VIR_FREE(info-devAlias[i]); VIR_FREE(info-devAlias); } + +/** + * virDomainInterfaceAddresses: + * @dom: domain object + * @ifaces: pointer to an array of pointers pointing to interface objects + * @flags: bitwise-OR of virDomainInterfaceAddressesFlags + * + * Return a pointer to the allocated array of pointers pointing to interfaces + * present in given domain along with their IP and MAC addresses. Note that + * single interface can have multiple or even 0 IP address. + * + * This API dynamically allocates the virDomainInterfacePtr struct based on + * how many interfaces domain @dom has, usually there's 1:1 correlation. The + * count of the interfaces is returned as the return value. + * + * In case @flags includes VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT, a configured + * guest agent is needed for successful return from this API. Moreover, if + * guest agent is used then the interface name is the one seen by guest OS. + * To match such interface with the one from @dom XML use MAC address or IP + * range. + * + * The lease-file parsing method returns the interface name of the form vnetN, + * which is different from what guest agent returns (like ethN or emN), and + * since the MAC address from guest agent might be different with what @dom XML + * specifies, we have no way to convert it into the names present in @dom + * config. Hence, it is not recommended to mix the flag ..._AGENT with + * ..._LEASE as it may lead to ambiguous results because we cannot be sure if + * the name came from the agent or from the other method. Reads
Re: [libvirt] [PATCH v2 0/4] attach-interface: Learn net type='direct'
On Fri, Feb 13, 2015 at 07:55:48AM -0500, Laine Stump wrote: On 02/12/2015 04:50 PM, John Ferlan wrote: On 02/09/2015 10:20 AM, Michal Privoznik wrote: v2 Michal Privoznik (4): libvirt_private.syms: Expose virDomainNetTypeFromString virsh attach-interface: Use enum instead of arbitrary integers virsh attach-interface: Use virDomainNetType{From,To}String() virsh attach-interface: Allow macvtap hotplug src/libvirt_private.syms | 1 + tools/virsh-domain.c | 36 ++-- tools/virsh.pod | 14 -- 3 files changed, 35 insertions(+), 16 deletions(-) ACK series... I had thought that during V1 of this patch series there had been enough sentiment about not pulling in private parts of libvirt internals that this V2 (posted 2 minutes before the last comment in V1, so very literally crossing in the inter-tubes) would be discarded in favor of something that only used public APIs - the last word on the subject was this: https://www.redhat.com/archives/libvir-list/2015-February/msg00284.html (Yes, I know there are other places where it's already done, but it seems like a better idea to clean those up than to compound the pollution.) Yeah, I'd prefer that in general, but this particular patch is only using the trivial VIR_ENUM string/int conversion, so I'm willing to overlook it for this. If the internal API being used were non-trivial then I'd say we should look at the public API support. 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] libxl and non-absolute paths
Just recently we moved to libvirt 1.2.12 for the next release. Which brought up a few problems when working with configs which we and Debian used to have. A mild complaint towards the xml validation: it would be really nice of that would be a bit more specific about what exactly it complains. It took me a while to realize that Extra element os in interleave was trying to tell me that the string of the loader element within the os section was not an absolute path. The issue here is that with libxl, I think the goal was to rather allow the library to select the path prefix (like for pygrub where the full path got removed recently). But now the xml validation disagrees. This would go for bootloader for xenpv and loader (within os) for xenfv. And for emulator in the device section. Though for that things are a bit more complicated. The libxl driver now calls that with the help option and decides from the output whether this is the traditional xen forked qemu or the upstream qemu binary. Then it selects the device model depending on that outcome. Not sure whether the libxl driver could query libxl for the path prefix. Right now the most straight forward way seems to move back to a full path for the emulator. At least now, by using the standard qemu binary for everything, we got a predictable path that does not change with Xen versions. So its possible to force migrate over to put /usr/bin/qemu-system-i386 there. But for loader and bootloader, do you think it reasonable to change the templates from absFilePath to filePath? -Stefan --- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng 2015-01-23 12:46:24. +++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 10:00:43.1616 @@ -258,7 +258,7 @@ /choice /attribute /optional -ref name=absFilePath/ +ref name=filePath/ /element /optional optional @@ -1060,7 +1060,7 @@ optional element name=bootloader choice -ref name=absFilePath/ +ref name=filePath/ empty/ /choice /element signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] numatune: Prefer old approach
On Thu, Feb 12, 2015 at 06:03:50PM +0100, Michal Privoznik wrote: Consider the following part of domain XML: numatune memory mode='static' nodeset=0,2/ /numatune cpu numa cell id='0' cpus='0' memory='65536' unit='KiB'/ /numa /cpu Yes, this have a great potential of breaking things. Especially, this will break migration between previous two or three upstream releases and current release we are working on, because libvirt started domains in more complicated way (even if not needed). After these patches, domains will be started in simpler way which is incompatible. On the other hand, we get backward compatibility with much more releases than we are about to break. So with this patch we get guaranteed migration compat for any configs which were valid under old libvirt. The new syntax will only ever be used if it is absolutely required, in which case migration wouldn't be supportable regardless. It is a shame we didn't think of doing tihs before, but I think I agree this is the right thing todo now. We're doomed either way, but this way feels like it minimises the doom to a smaller set of people. 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
Re: [libvirt] [PATCH v8 2/4] domifaddr: Implement the remote protocol
On 01/25/2015 01:38 PM, Nehal J Wani wrote: daemon/remote.c * Define remoteSerializeDomainInterface, remoteDispatchDomainInterfaceAddresses src/remote/remote_driver.c * Define remoteDomainInterfaceAddresses src/remote/remote_protocol.x * New RPC procedure: REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES * Define structs remote_domain_ip_addr, remote_domain_interface, remote_domain_interfaces_addresse_args, remote_domain_interface_addresses_ret * Introduce upper bounds (to handle DoS attacks): REMOTE_DOMAIN_INTERFACE_MAX = 2048 REMOTE_DOMAIN_IP_ADDR_MAX = 2048 Restrictions on the maximum number of aliases per interface were removed after kernel v2.0, and theoretically, at present, there are no upper limits on number of interfaces per virtual machine and on the number of IP addresses per interface. src/remote_protocol-structs * New structs added Signed-off-by: Nehal J Wani nehaljw.k...@gmail.com --- daemon/remote.c | 134 +++ src/libvirt_public.syms | 4 +- src/remote/remote_driver.c | 100 src/remote/remote_protocol.x | 36 +++- src/remote_protocol-structs | 24 5 files changed, 295 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d657a09..32b567c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6438,6 +6438,140 @@ remoteDispatchDomainGetFSInfo(virNetServerPtr server ATTRIBUTE_UNUSED, } +static int +remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces, + unsigned int ifaces_count, + remote_domain_interface_addresses_ret *ret) +{ +size_t i, j; + +if (ifaces_count REMOTE_DOMAIN_INTERFACE_MAX) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Number of interfaces, %d exceeds the max limit: %d), + ifaces_count, REMOTE_DOMAIN_INTERFACE_MAX); +return -1; +} + +if (VIR_ALLOC_N(ret-ifaces.ifaces_val, ifaces_count) 0) +return -1; Something is not right here... Coverity squawks later on [1] + +ret-ifaces.ifaces_len = ifaces_count; + +for (i = 0; i ifaces_count; i++) { +virDomainInterfacePtr iface = ifaces[i]; +remote_domain_interface *iface_ret = (ret-ifaces.ifaces_val[i]); + +if ((VIR_STRDUP(iface_ret-name, iface-name)) 0) +goto cleanup; + +if (iface-hwaddr) { +char **hwaddr_p = NULL; +if (VIR_ALLOC(hwaddr_p) 0) +goto cleanup; +if (VIR_STRDUP(*hwaddr_p, iface-hwaddr) 0) { +VIR_FREE(hwaddr_p); +goto cleanup; +} so hw_addrp is an allocated array of one element of which the [0] is allocated (strdup'd); however, there's only ever one free... Should this be something like: if (iface-hwaddr) { if (VIR_ALLOC_N(iface_ret-hwaddr, strlen(iface-hwaddr) + 1) 0) goto cleanup; memcpy(iface_ret-hwaddr, iface-hwaddr, strlen(iface-hwaddr)); } ? + +iface_ret-hwaddr = hwaddr_p; +} + +if (iface-naddrs REMOTE_DOMAIN_IP_ADDR_MAX) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Number of interfaces, %d exceeds the max limit: %d), + iface-naddrs, REMOTE_DOMAIN_IP_ADDR_MAX); +goto cleanup; +} + +if (VIR_ALLOC_N(iface_ret-addrs.addrs_val, +iface-naddrs) 0) +goto cleanup; + +iface_ret-addrs.addrs_len = iface-naddrs; + +for (j = 0; j iface-naddrs; j++) { +virDomainIPAddressPtr ip_addr = (iface-addrs[j]); +remote_domain_ip_addr *ip_addr_ret = +(iface_ret-addrs.addrs_val[j]); + +if (VIR_STRDUP(ip_addr_ret-addr, ip_addr-addr) 0) +goto cleanup; + +ip_addr_ret-prefix = ip_addr-prefix; +ip_addr_ret-type = ip_addr-type; +} +} + +return 0; + + cleanup: +if (ret-ifaces.ifaces_val) { +for (i = 0; i ifaces_count; i++) { +remote_domain_interface *iface_ret = (ret-ifaces.ifaces_val[i]); +VIR_FREE(iface_ret-name); +VIR_FREE(iface_ret-hwaddr); hwaddr was a VIR_ALLOC then VIR_STRDUP(*iface_ret-hwaddr)... so this would leak the strdup'd string unless you change to perhaps _N model. +for (j = 0; j iface_ret-addrs.addrs_len; j++) { +remote_domain_ip_addr *ip_addr = +(iface_ret-addrs.addrs_val[j]); +VIR_FREE(ip_addr-addr); +} +VIR_FREE(iface_ret); ^^^[1] Coverity points out that iface_ret is an incorrect free since it wasn't allocated separately The ifaces_val is an VIR_ALLOC_N,
Re: [libvirt] [PATCH v3 2/2] security: Refactor virSecurityManagerGenLabel
On Thu, Feb 12, 2015 at 06:32:41PM +0100, Erik Skultety wrote: if (mgr == NULL || mgr-drv == NULL) return ret; This check isn't really necessary, security manager cannot be a NULL pointer as it is either selinux (by default) or 'none', if no other driver is set in the config. Even with no config file driver name yields 'none'. The other hunk checks for domain's security model validity, but we should also check devices' security model as well, therefore this hunk is moved into a separate function which is called by virSecurityManagerCheckAllLabel that checks both the domain's security model and devices' security model. https://bugzilla.redhat.com/show_bug.cgi?id=1165485 --- src/security/security_manager.c | 41 ++--- 1 file changed, 22 insertions(+), 19 deletions(-) ACK @@ -731,6 +713,22 @@ static int virSecurityManagerCheckSecurityModel(char *secmodel, static int +virSecurityManagerCheckSecurityDomainLabel(virDomainDefPtr def, + void *opaque) Same comments as for v1 regarding the use of void and the repeating extra word. @@ -776,6 +774,11 @@ int virSecurityManagerCheckAllLabel(virSecurityManagerPtr mgr, { size_t i; +/* first check per-domain seclabels */ These comments don't seem very helpful - a function named CheckDomainLabel should do exactly that. I fixed the nits and pushed the patch. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] security: introduce virSecurityManagerCheckAllLabel function
On Thu, Feb 12, 2015 at 06:32:40PM +0100, Erik Skultety wrote: We do have a check for valid per-domain security model, however we still do permit an invalid security model for a domain's device (those which are specified with source element). This patch introduces a new function virSecurityManagerCheckAllLabel which compares user specified security model against currently registered security drivers. That being said, it also permits 'none' being specified as a device security model. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1165485 --- src/libvirt_private.syms| 1 + src/lxc/lxc_process.c | 3 ++ src/qemu/qemu_process.c | 6 +++ src/security/security_manager.c | 89 + src/security/security_manager.h | 2 + 5 files changed, 101 insertions(+) ACK +static int virSecurityManagerCheckSecurityModel(char *secmodel, +void *opaque) Only callbacks should use void *opaque. The redundant 'Security' occurs twice in the function names. I fixed the parameter types, and removed the extra word to save some screen space and pushed the patch. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/6] qemuMigrationDriveMirror: Listen to events
https://bugzilla.redhat.com/show_bug.cgi?id=1179678 When migrating with storage, libvirt iterates over domain disks and instruct qemu to migrate the ones we are interested in (shared, RO and source-less disks are skipped). The disks are migrated in series. No new disk is transferred until the previous one hasn't been quiesced. This is checked on the qemu monitor via 'query-jobs' command. If the disk has been quiesced, it practically went from copying its content to mirroring state, where all disk writes are mirrored to the other side of migration too. Having said that, there's one inherent error in the design. The monitor command we use reports only active jobs. So if the job fails for whatever reason, we will not see it anymore in the command output. And this can happen fairly simply: just try to migrate a domain with storage. If the storage migration fails (e.g. due to ENOSPC on the destination) we resume the host on the destination and let it run on partly copied disk. The proper fix is what even the comment in the code says: listen for qemu events instead of polling. If storage migration changes state an event is emitted and we can act accordingly: either consider disk copied and continue the process, or consider disk mangled and abort the migration. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_migration.c | 37 + 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4506d87..14a4ec6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1739,7 +1739,6 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, for (i = 0; i vm-def-ndisks; i++) { virDomainDiskDefPtr disk = vm-def-disks[i]; -virDomainBlockJobInfo info; /* skip shared, RO and source-less disks */ if (disk-src-shared || disk-src-readonly || @@ -1772,38 +1771,36 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, /* Poll every 500ms for progress to allow cancellation */ struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000 * 1000ull }; -memset(info, 0, sizeof(info)); - -if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) 0) +/* Explicitly check if domain is still alive. Maybe qemu + * died meanwhile so we won't see any event at all. */ +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(guest unexpectedly quit)); goto error; +} + +/* The following check should be race free as long as the variable + * is set only with domain object locked. And here we have the + * domain object locked too. */ if (priv-job.asyncAbort) { -/* explicitly do this *after* we entered the monitor, - * as this is a critical section so we are guaranteed - * priv-job.asyncAbort will not change */ -ignore_value(qemuDomainObjExitMonitor(driver, vm)); priv-job.current-type = VIR_DOMAIN_JOB_CANCELLED; virReportError(VIR_ERR_OPERATION_ABORTED, _(%s: %s), qemuDomainAsyncJobTypeToString(priv-job.asyncJob), _(canceled by client)); goto error; } -mon_ret = qemuMonitorBlockJobInfo(priv-mon, diskAlias, info, - NULL); -if (qemuDomainObjExitMonitor(driver, vm) 0) -goto error; -if (mon_ret 0) -goto error; - -if (info.cur == info.end) { +if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_READY) { VIR_DEBUG(Drive mirroring of '%s' completed, diskAlias); break; +} else if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) { +virReportError(VIR_ERR_OPERATION_FAILED, + _(migration of disk %s failed), + disk-dst); +goto error; } -/* XXX Frankly speaking, we should listen to the events, - * instead of doing this. But this works for now and we - * are doing something similar in migration itself anyway */ +/* XXX Turn this into virCond someday. */ virObjectUnlock(vm); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/6] qemuProcessHandleBlockJob: Set disk-mirrorState more often
Currently, upon BLOCK_JOB_* event, disk-mirrorState is not updated each time. The callback code handling the events checks if a blockjob was started via our public APIs prior to setting the mirrorState. However, some block jobs may be started internally (e.g. during storage migration), in which case we don't bother with setting disk-mirror (there's nothing we can set it to anyway), or other fields. But it will come handy if we update the mirrorState in these cases too. The event wasn't delivered just for fun - we've started the job after all. So, in this commit, the mirrorState is set to whatever job status we've obtained. Of course, there are some actions on some statuses that we want to perform. But instead of if {} else if {} else {} ... enumeration, let's move to switch(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_process.c | 35 --- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 43a64a1..f30acbb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1041,7 +1041,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, /* If we completed a block pull or commit, then update the XML * to match. */ -if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) { +switch ((virConnectDomainEventBlockJobStatus) status) { +case VIR_DOMAIN_BLOCK_JOB_COMPLETED: if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { if (vm-newDef) { int indx = virDomainDiskIndexByName(vm-newDef, disk-dst, @@ -1091,20 +1092,24 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true, true)); -} else if (disk-mirror - (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY || -type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) { -if (status == VIR_DOMAIN_BLOCK_JOB_READY) { -disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; -save = true; -} else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED || - status == VIR_DOMAIN_BLOCK_JOB_CANCELED) { -virStorageSourceFree(disk-mirror); -disk-mirror = NULL; -disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; -disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; -save = true; -} +break; + +case VIR_DOMAIN_BLOCK_JOB_READY: +disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; +save = true; +break; + +case VIR_DOMAIN_BLOCK_JOB_FAILED: +case VIR_DOMAIN_BLOCK_JOB_CANCELED: +virStorageSourceFree(disk-mirror); +disk-mirror = NULL; +disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; +save = true; +break; + +case VIR_DOMAIN_BLOCK_JOB_LAST: +break; } } -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/6] qemuDomainObjPrivate: Introduce blockJob condition
So far the condition is not used, but will be. There are some operations, where we actively wait for a block job to complete. Instead of locking and unlocking the domain object, entering and leaving monitor, lets just use a condition and let our monitor event handling code wake up when needed. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_domain.c | 4 +++- src/qemu/qemu_domain.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..28961d2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -406,7 +406,8 @@ qemuDomainObjPrivateAlloc(void) goto error; } -if (virCondInit(priv-unplugFinished) 0) +if (virCondInit(priv-unplugFinished) 0 || +virCondInit(priv-blockJob) 0) goto error; if (!(priv-devs = virChrdevAlloc())) @@ -439,6 +440,7 @@ qemuDomainObjPrivateFree(void *data) VIR_FREE(priv-origname); virCondDestroy(priv-unplugFinished); +virCondDestroy(priv-blockJob); virChrdevFree(priv-devs); /* This should never be non-NULL if we get here, but just in case... */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2c3881..db9ffac 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -186,6 +186,8 @@ struct _qemuDomainObjPrivate { const char *unpluggingDevice; /* alias of the device that is being unplugged */ char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */ +virCond blockJob; /* signals that one of disks translated state of a block job */ + bool hookRun; /* true if there was a hook run over this domain */ virBitmapPtr autoNodeset; }; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 6/6] qemuDomainBlockJobImpl: utilize blockJob condition
Instead of unlocking and locking the domain object every 50ms lets just wait on blockJob condition and run the loop body if and BLOCK_JOB even occurred. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- This one, well, I still left the only monitor call as I'm not very familiar with this code so I don't know if it can be more optimized. But hey, the 50ms sleep is gone! src/qemu/qemu_driver.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 709f468..9298619 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15872,10 +15872,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, /* XXX If the event reports failure, we should reflect * that back into the return status of this API call. */ while (1) { -/* Poll every 50ms */ -static struct timespec ts = { -.tv_sec = 0, -.tv_nsec = 50 * 1000 * 1000ull }; virDomainBlockJobInfo dummy; qemuDomainObjEnterMonitor(driver, vm); @@ -15886,11 +15882,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, if (ret = 0) break; -virObjectUnlock(vm); - -nanosleep(ts, NULL); - -virObjectLock(vm); +if (virCondWait(priv-blockJob, vm-parent.lock) 0) { +virReportSystemError(errno, %s, + _(Unable to wait on blockJob condition)); +ret = -1; +break; +} if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, %s, -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 5/6] qemuMigrationDriveMirror: utilize blockJob condition
Instead of unlocking and locking the domain object every 50ms lets just wait on blockJob condition and run the loop body if and BLOCK_JOB even occurred. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_migration.c | 15 +-- src/qemu/qemu_process.c | 4 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 14a4ec6..998e8f5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1768,9 +1768,6 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, /* wait for completion */ while (true) { -/* Poll every 500ms for progress to allow cancellation */ -struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000 * 1000ull }; - /* Explicitly check if domain is still alive. Maybe qemu * died meanwhile so we won't see any event at all. */ if (!virDomainObjIsActive(vm)) { @@ -1800,13 +1797,11 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, goto error; } -/* XXX Turn this into virCond someday. */ - -virObjectUnlock(vm); - -nanosleep(ts, NULL); - -virObjectLock(vm); +if (virCondWait(priv-blockJob, vm-parent.lock) 0) { +virReportSystemError(errno, %s, + _(Unable to wait on blockJob condition)); +goto error; +} } } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9cfc0f3..7b87cdb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1017,6 +1017,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, void *opaque) { virQEMUDriverPtr driver = opaque; +qemuDomainObjPrivatePtr priv; virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; const char *path; @@ -1026,6 +1027,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, bool save = false; virObjectLock(vm); +priv = vm-privateData; disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); if (disk) { @@ -1112,6 +1114,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, case VIR_DOMAIN_BLOCK_JOB_LAST: break; } + +virCondSignal(priv-blockJob); } if (save) { -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/6] Propagate storage errors on migration
diff to v3: - Took a step further and introduced a condition to wait on Michal Privoznik (6): qemuProcessHandleBlockJob: Set disk-mirrorState more often qemuProcessHandleBlockJob: Take status into account qemuMigrationDriveMirror: Listen to events qemuDomainObjPrivate: Introduce blockJob condition qemuMigrationDriveMirror: utilize blockJob condition qemuDomainBlockJobImpl: utilize blockJob condition src/qemu/qemu_domain.c| 4 +++- src/qemu/qemu_domain.h| 2 ++ src/qemu/qemu_driver.c| 15 ++ src/qemu/qemu_migration.c | 50 --- src/qemu/qemu_process.c | 40 +++-- 5 files changed, 57 insertions(+), 54 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/6] qemuProcessHandleBlockJob: Take status into account
Upon BLOCK_JOB_COMPLETED event delivery, we check if the job has completed (in qemuMonitorJSONHandleBlockJobImpl()). For better image, the event looks something like this: timestamp: {seconds: 1423582694, microseconds: 372666}, event: BLOCK_JOB_COMPLETED, data: {device: drive-virtio-disk0, len: 8412790784, offset: 409993216, speed: 8796093022207, type: mirror, error: No space left on device}} If len does not equal offset it's considered an error, and we can clearly see error field filled in. However, later in the event processing this case was handled no differently to case of job being aborted via separate API. It's time that we start differentiate these two because of the future work. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f30acbb..9cfc0f3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1103,7 +1103,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, case VIR_DOMAIN_BLOCK_JOB_CANCELED: virStorageSourceFree(disk-mirror); disk-mirror = NULL; -disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +disk-mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ? +VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; save = true; break; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 3/4] domifaddr: Implement the API for qemu
On 01/25/2015 01:38 PM, Nehal J Wani wrote: By querying the qemu guest agent with the QMP command guest-network-get-interfaces and converting the received JSON output to structured objects. Although ifconfig is deprecated, IP aliases created by ifconfig are supported by this API. The legacy syntax of an IP alias is: ifname:alias-name. Since we want all aliases to be clubbed under parent interface, simply stripping :alias-name suffices. Note that IP aliases formed by ip aren't visible to ifconfig, and aliases created by ip do not have any specific name. But we are lucky, as qemu guest agent detects aliases created by both. src/qemu/qemu_agent.h: * Define qemuAgentGetInterfaces src/qemu/qemu_agent.c: * Implement qemuAgentGetInterface src/qemu/qemu_driver.c: * New function qemuGetDHCPInterfaces * New function qemuDomainInterfaceAddresses src/remote_protocol-sructs: * Define new structs tests/qemuagenttest.c: * Add new test: testQemuAgentGetInterfaces Test cases for IP aliases, 0 or multiple ipv4/ipv6 address(es) Signed-off-by: Nehal J Wani nehaljw.k...@gmail.com --- src/qemu/qemu_agent.c | 202 + src/qemu/qemu_agent.h | 4 + src/qemu/qemu_driver.c | 173 ++ tests/qemuagenttest.c | 188 + 4 files changed, 567 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5fcc40f..e881cdc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1953,3 +1953,205 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentGetInterfaces: + * @mon: Agent monitor + * @ifaces: pointer to an array of pointers pointing to interface objects + * + * Issue guest-network-get-interfaces to guest agent, which returns a + * list of interfaces of a running domain along with their IP and MAC + * addresses. + * + * Returns: number of interfaces on success, -1 on error. + */ +int +qemuAgentGetInterfaces(qemuAgentPtr mon, + virDomainInterfacePtr **ifaces) +{ +int ret = -1; +size_t i, j; +int size = -1; +virJSONValuePtr cmd = NULL; +virJSONValuePtr reply = NULL; +virJSONValuePtr ret_array = NULL; +size_t ifaces_count = 0; +size_t addrs_count = 0; +virDomainInterfacePtr *ifaces_ret = NULL; +virHashTablePtr ifaces_store = NULL; +char **ifname = NULL; + +/* Hash table to handle the interface alias */ +if (!(ifaces_store = virHashCreate(ifaces_count, NULL))) { +virHashFree(ifaces_store); +return -1; +} + +if (!(cmd = qemuAgentMakeCommand(guest-network-get-interfaces, NULL))) +goto cleanup; + +if (qemuAgentCommand(mon, cmd, reply, false, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0 || +qemuAgentCheckError(cmd, reply) 0) { +goto cleanup; +} + +if (!(ret_array = virJSONValueObjectGet(reply, return))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent didn't provide 'return' field)); +goto cleanup; +} + +if ((size = virJSONValueArraySize(ret_array)) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent didn't return an array of interfaces)); +goto cleanup; +} + +for (i = 0; i size; i++) { +virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); +virJSONValuePtr ip_addr_arr = NULL; +const char *hwaddr, *ifname_s, *name = NULL; +int ip_addr_arr_size; +virDomainInterfacePtr iface = NULL; + +/* Shouldn't happen but doesn't hurt to check neither */ +if (!tmp_iface) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(something has went really wrong)); +goto error; +} + +/* interface name is required to be presented */ +name = virJSONValueObjectGetString(tmp_iface, name); +if (!name) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent didn't provide 'name' field)); +goto error; +} + +/* Handle interface alias (ifname:alias) */ +ifname = virStringSplit(name, :, 2); +ifname_s = ifname[0]; + +iface = virHashLookup(ifaces_store, ifname_s); + +/* If the hash table doesn't contain this iface, add it */ +if (!iface) { +if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) 0) +goto error; + +if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) 0) +goto error; + +if (virHashAddEntry(ifaces_store, ifname_s, +ifaces_ret[ifaces_count - 1]) 0) +goto error; + +
Re: [libvirt] [PATCH v8 4/4] domifaddr: Add virsh support
On 01/25/2015 01:38 PM, Nehal J Wani wrote: tools/virsh-domain-monitor.c * Introduce new command : domifaddr Usage: domifaddr domain [interface] [--full] [--lease] [--agent] Example outputs: virsh # domifaddr f20 Name MAC address Protocol Address --- lo 00:00:00:00:00:00ipv4 127.0.0.1/8 - -ipv6 ::1/128 eth0 52:54:00:2e:45:ceipv4 10.1.33.188/24 - -ipv6 2001:db8:0:f101::2/64 - -ipv6 fe80::5054:ff:fe2e:45ce/64 eth1 52:54:00:b1:70:19ipv4 192.168.105.201/16 - -ipv4 192.168.201.195/16 - -ipv6 fe80::5054:ff:feb1:7019/64 eth2 52:54:00:36:2a:e5N/A N/A eth3 52:54:00:20:70:3dipv4 192.168.105.240/16 - -ipv6 fe80::5054:ff:fe20:703d/64 virsh # domifaddr f20 eth1 --agent Name MAC address Protocol Address --- eth1 52:54:00:b1:70:19ipv4 192.168.105.201/16 - -ipv4 192.168.201.195/16 - -ipv6 fe80::5054:ff:feb1:7019/64 virsh # domifaddr f20 eth0 --agent --full Name MAC address Protocol Address --- eth0 52:54:00:2e:45:ceipv4 10.1.33.188/24 eth0 52:54:00:2e:45:ceipv6 2001:db8:0:f101::2/64 eth0 52:54:00:2e:45:ceipv6 fe80::5054:ff:fe2e:45ce/64 tools/virsh.pod * Document new command I for one like the format of the output and appreciate seeing it in the commit message! Signed-off-by: Nehal J Wani nehaljw.k...@gmail.com --- tools/virsh-domain-monitor.c | 141 +++ tools/virsh.pod | 16 + 2 files changed, 157 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 925eb1b..960b831 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2177,6 +2177,141 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) return ret; } +/* domifaddr command + */ +static const vshCmdInfo info_domifaddr[] = { +{help, N_(Get network interfaces' addresses for a running domain)}, +{desc, N_(Get network interfaces' addresses for a running domain)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_domifaddr[] = { +{.name = domain, + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_(domain name, id or uuid)}, +{.name = interface, + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_NONE, + .help = N_(network interface name)}, +{.name = full, + .type = VSH_OT_BOOL, + .flags = VSH_OFLAG_NONE, + .help = N_(display full fields)}, +{.name = lease, + .type = VSH_OT_BOOL, + .flags = VSH_OFLAG_NONE, + .help = N_(parse dhcp lease file)}, +{.name = agent, + .type = VSH_OT_BOOL, + .flags = VSH_OFLAG_NONE, + .help = N_(query qemu guest agent)}, +{.name = NULL} +}; + +static bool +cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *interface = NULL; +virDomainInterfacePtr *ifaces = NULL; +size_t i, j; +int ifaces_count = 0; +unsigned int flags = 0; +bool ret = false; +bool full = vshCommandOptBool(cmd, full); + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +if (vshCommandOptString(cmd, interface, interface) 0) +goto cleanup; + +if (vshCommandOptBool(cmd, lease)) +flags |= VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE; + +if (vshCommandOptBool(cmd, agent)) +flags |= VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT; + Since only one style would be displayed (lease in preference over agent) - providing both flags could be an error... Unless of course you change 3/4 to allow tryAgent after tryLease returns. +if ((ifaces_count = virDomainInterfaceAddresses(dom, ifaces, flags)) 0) { [1] Same Coverity issue as 2/4 +vshError(ctl, _(Failed to query for interfaces addresses)); +goto cleanup; +} + +vshPrintExtra(ctl, %-10s %-20s %-8s %s\n%s%s\n, _(Name), + _(MAC address), _(Protocol), _(Address), + _(-), + _(--)); + +for (i = 0; i ifaces_count; i++) { +
[libvirt] [PATCH 1/2] security: Resolve Coverity RESOURCE_LEAK
Introduced by commit id 'c3d9d3bb' - return from virSecurityManagerCheckModel wasn't VIR_FREE()'ing the virSecurityManagerGetNested allocated memory. Signed-off-by: John Ferlan jfer...@redhat.com --- src/security/security_manager.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index f2a32bc..12663ad 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -688,24 +688,31 @@ virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, static int virSecurityManagerCheckModel(virSecurityManagerPtr mgr, char *secmodel) { +int ret = -1; size_t i; virSecurityManagerPtr *sec_managers = NULL; if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL) return -1; -if (STREQ_NULLABLE(secmodel, none)) -return 0; +if (STREQ_NULLABLE(secmodel, none)) { +ret = 0; +goto cleanup; +} for (i = 0; sec_managers[i]; i++) { -if (STREQ_NULLABLE(secmodel, sec_managers[i]-drv-name)) -return 0; +if (STREQ_NULLABLE(secmodel, sec_managers[i]-drv-name)) { +ret = 0; +goto cleanup; +} } virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(Unable to find security driver for model %s), secmodel); -return -1; + cleanup: +VIR_FREE(sec_managers); +return ret; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Resolve a couple of Coverity issues
One new one and one that I've seen periodically. John Ferlan (2): security: Resolve Coverity RESOURCE_LEAK libxl: Resolve Coverity CHECKED_RETURN src/libxl/libxl_domain.c| 6 +- src/security/security_manager.c | 17 - 2 files changed, 17 insertions(+), 6 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] libxl: Resolve Coverity CHECKED_RETURN
Periodically my Coverity scan will return a checked_return failure for libxlDomainShutdownThread call to libxlDomainStart. Followed the libxlAutostartDomain example in order to check the status, emit a message, and continue on. Signed-off-by: John Ferlan jfer...@redhat.com --- src/libxl/libxl_domain.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index f0eaf6c..21c41d7 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -685,7 +685,11 @@ libxlDomainShutdownThread(void *opaque) } libxl_domain_destroy(ctx, vm-def-id, NULL); libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); -libxlDomainStart(driver, vm, 0, -1); +if (libxlDomainStart(driver, vm, false, -1) 0) { +virErrorPtr err = virGetLastError(); +VIR_ERROR(_(Failed to restart VM '%s': %s), + vm-def-name, err ? err-message : _(unknown error)); +} cleanup: if (vm) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Comment out variables/functions that are unused.
Avoids complaints when the compiler is configured to warn-unused. A few files contain unnecessary code that results in the compiler erroring out when -Wunused* options are used. Comment out the code until such time as it is needed. --- src/libxl/libxl_conf.c | 2 ++ tests/virnetsockettest.c | 8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0555b91..f8db4d2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -305,7 +305,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) regmatch_t subs[4]; char *saveptr = NULL; size_t i; +/* virArch hostarch = caps-host.arch; +*/ struct guest_arch guest_archs[32]; int nr_guest_archs = 0; diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 5d91f26..988ab43 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -333,9 +333,10 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) return ret; } +/* static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / char buf[100]; size_t i; int ret = -1; @@ -360,10 +361,12 @@ static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) virObjectUnref(csock); return ret; } +*/ +/* static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / char buf[100]; int ret = -1; virCommandPtr cmd = virCommandNewArgList(/bin/cat, /dev/does-not-exist, NULL); @@ -383,6 +386,7 @@ static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) virObjectUnref(csock); return ret; } +*/ struct testSSHData { const char *nodename; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/3] lxc: fix show the wrong xml when guest start failed
On 02/12/2015 03:48 PM, John Ferlan wrote: This is a rework of the patch Luyao Huang sent to resolve a couple of issues found in virLXCStartProcess. See explanation here: http://www.redhat.com/archives/libvir-list/2015-February/msg00433.html John Ferlan (1): lxc: Modify/add some debug messages Luyao Huang (2): lxc: Move console checks in LXCProcessStart lxc: Fix container cleanup for LXCProcessStart src/lxc/lxc_process.c | 109 -- 1 file changed, 52 insertions(+), 57 deletions(-) Thanks for the quick review - now pushed. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Fwd: libvirt 1.2.10 and latest EL6 qemu-kvm
Since there was no reaction in the user list, I'm posting it in the devel in the hope for some guidance to the problem mentioned below (sorry about this). Franky -- Forwarded message -- From: Franky Van Liedekerke fra...@e-dynamics.be Date: Wed, Feb 11, 2015 at 3:44 PM Subject: Re: libvirt 1.2.10 and latest EL6 qemu-kvm To: libvirt-us...@redhat.com (I'm sorry, I didn't find my original message but I provide the archive link: https://www.redhat.com/archives/libvirt-users/2015-January/msg00069.html ) I just tried the latest libvirt release (1.2.12), combined with the following qemu EL6 package from centos: qemu-kvm-0.12.1.2-2.448.el6_6.x86_64 Using 1.2.12 and this version of qemu-kvm, I still get the following error when trying to start a domain: error: Failed to start domain btcab0001ap.unix.banksys.be error: internal error: unable to execute QEMU command 'qom-list': The command qom-list has not been found The older qemu package (qemu-kvm-0.12.1.2-2.415.el6_5.14.x86_64) works just fine though (I also tried the qemu-rhev-packages for el6 from ovirt, but no change). I verified the changelog from libvirt 1.2.12 and the earlier suggested fix is in there (but doesn't seem to change anything): 2015-01-14 Pavel Hrdina phrd...@redhat.com qemu_monitor: introduce new function to get QOM path Now, I have activated libvirtd debug and these are the qom-list lines (I can provide the full logfile if wanted): 2015-02-11 14:10:33.259+: 13280: debug : virJSONValueToString:1528 : result={execute:qom-list,arguments:{path:/machine/unattached/device[0]},id:libvirt-5} 2015-02-11 14:10:33.259+: 13280: debug : qemuMonitorJSONCommandWithFd:290 : Send command '{execute:qom-list,arguments:{path:/machine/unattached/device[0]},id:libvirt-5}' for write with FD -1 2015-02-11 14:10:33.259+: 13280: info : qemuMonitorSend:972 : QEMU_MONITOR_SEND_MSG: mon=0x7fa62801cbf0 msg={execute:qom-list,arguments:{path:/machine/unattached/device[0]},id:libvirt-5} 2015-02-11 14:10:33.260+: 13278: info : qemuMonitorIOWrite:503 : QEMU_MONITOR_IO_WRITE: mon=0x7fa62801cbf0 buf={execute:qom-list,arguments:{path:/machine/unattached/device[0]},id:libvirt-5} 2015-02-11 14:10:33.261+: 13278: info : qemuMonitorIOProcess:399 : QEMU_MONITOR_IO_PROCESS: mon=0x7fa62801cbf0 buf={id: libvirt-5, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}} 2015-02-11 14:10:33.261+: 13278: debug : qemuMonitorJSONIOProcessLine:183 : Line [{id: libvirt-5, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}}] 2015-02-11 14:10:33.261+: 13278: debug : virJSONValueFromString:1361 : string={id: libvirt-5, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}} 2015-02-11 14:10:33.261+: 13278: info : qemuMonitorJSONIOProcessLine:203 : QEMU_MONITOR_RECV_REPLY: mon=0x7fa62801cbf0 reply={id: libvirt-5, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}} 2015-02-11 14:10:33.289+: 13280: debug : virJSONValueToString:1528 : result={execute:qom-list,arguments:{path:/machine/peripheral},id:libvirt-7} 2015-02-11 14:10:33.289+: 13280: debug : qemuMonitorJSONCommandWithFd:290 : Send command '{execute:qom-list,arguments:{path:/machine/peripheral},id:libvirt-7}' for write with FD -1 2015-02-11 14:10:33.289+: 13280: info : qemuMonitorSend:972 : QEMU_MONITOR_SEND_MSG: mon=0x7fa62801cbf0 msg={execute:qom-list,arguments:{path:/machine/peripheral},id:libvirt-7} 2015-02-11 14:10:33.289+: 13278: info : qemuMonitorIOWrite:503 : QEMU_MONITOR_IO_WRITE: mon=0x7fa62801cbf0 buf={execute:qom-list,arguments:{path:/machine/peripheral},id:libvirt-7} 2015-02-11 14:10:33.291+: 13278: info : qemuMonitorIOProcess:399 : QEMU_MONITOR_IO_PROCESS: mon=0x7fa62801cbf0 buf={id: libvirt-7, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}} 2015-02-11 14:10:33.291+: 13278: debug : qemuMonitorJSONIOProcessLine:183 : Line [{id: libvirt-7, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}}] 2015-02-11 14:10:33.291+: 13278: debug : virJSONValueFromString:1361 : string={id: libvirt-7, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}} 2015-02-11 14:10:33.291+: 13278: info : qemuMonitorJSONIOProcessLine:203 : QEMU_MONITOR_RECV_REPLY: mon=0x7fa62801cbf0 reply={id: libvirt-7, error: {class: CommandNotFound, desc: The command qom-list has not been found, data: {name: qom-list}}} 2015-02-11 14:10:33.291+: 13280: debug : virJSONValueToString:1528 : result={execute:qom-list,arguments:{path:/machine/peripheral},id:libvirt-7} 2015-02-11 14:10:33.292+: 13280: debug : virJSONValueToString:1528 : result={id:libvirt-7,error:{class:CommandNotFound,desc:The command qom-list has not
[libvirt] [PATCH v2] Comment out variables/functions that are unused.
Avoids complaints when the compiler is configured to warn-unused. A few files contain unnecessary code that results in the compiler erroring out when -Wunused* options are used. Comment out the code until such time as it is needed. Signed-off-by: Gary R Hook gary.h...@nimboxx.com --- src/libxl/libxl_conf.c | 2 ++ tests/virnetsockettest.c | 8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0555b91..f8db4d2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -305,7 +305,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) regmatch_t subs[4]; char *saveptr = NULL; size_t i; +/* virArch hostarch = caps-host.arch; +*/ struct guest_arch guest_archs[32]; int nr_guest_archs = 0; diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 5d91f26..988ab43 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -333,9 +333,10 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) return ret; } +/* static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / char buf[100]; size_t i; int ret = -1; @@ -360,10 +361,12 @@ static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) virObjectUnref(csock); return ret; } +*/ +/* static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / char buf[100]; int ret = -1; virCommandPtr cmd = virCommandNewArgList(/bin/cat, /dev/does-not-exist, NULL); @@ -383,6 +386,7 @@ static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) virObjectUnref(csock); return ret; } +*/ struct testSSHData { const char *nodename; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list