Re: [libvirt] [PATCH] openvz: Handle domain obj hash map errors
On 2012年07月10日 13:21, Guido Günther wrote: This makes the driver fail with a clear error message in case of uuid collisions (for example if somebody copied a container configuration without updating the UUID). OpenVZ itself doesn't complain about duplicate UUIDs since this parameter is only used by libvirt. --- src/openvz/openvz_conf.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 503c8a0..0764c2c 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -684,8 +684,11 @@ int openvzLoadDomains(struct openvz_driver *driver) { openvzReadMemConf(dom-def, veid); virUUIDFormat(dom-def-uuid, uuidstr); -if (virHashAddEntry(driver-domains.objs, uuidstr, dom) 0) +if (virHashAddEntry(driver-domains.objs, uuidstr, dom) 0) { +openvzError(VIR_ERR_INTERNAL_ERROR, +_(Could not add UUID for container %d), veid); goto cleanup; +} It's good to have a clear error for the UUID collisions, but it's not the only reason for failure of virHashAddEntry. It could be the memory allocation error too. So The better fix is to check the UUID duplication explicitly with virHashLookup before adding it to the table, (virHashAddOrUpdateEntry reports the memory allocation error anyway, so we just need to care about the UUID conllisions, and actually the memory error could be overriden with this patch). Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] domain_conf: Add USB controler model none
On 2012年07月10日 01:29, Peter Krempa wrote: Libvirt adds a USB controller to the guest even if the user does not specify any in the XML. This is due to back-compat reasons. It's not for back-compat reasons as far as I known, 42043afcd adds the implicit USB controller for virt-manager's use, and it causes the regression bug of migration, as the old libvirt doesn't have a default USB controller as a force. Jirka fixed it by commit 409b5f549. To allow disabling USB for a guest this patch adds a new USB controller type none that disables USB support for the guest. --- docs/schemas/domaincommon.rng |1 + src/conf/domain_conf.c| 55 - src/conf/domain_conf.h|1 + src/qemu/qemu_command.c |3 +- 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3d205b0..937c2e8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1202,6 +1202,7 @@ valuevt82c686b-uhci/value valuepci-ohci/value valuenec-xhci/value +valuenone/value /choice /attribute /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 17b0e0e..9afae87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -257,7 +257,8 @@ VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, ich9-uhci3, vt82c686b-uhci, pci-ohci, - nec-xhci) + nec-xhci, + none) VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, mount, @@ -7910,6 +7911,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virBitmapPtr bootMap = NULL; unsigned long bootMapSize = 0; xmlNodePtr cur; +bool usb_none = false; +bool usb_other = false; if (VIR_ALLOC(def) 0) { virReportOOMError(); @@ -8635,6 +8638,27 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!controller) goto error; +/* sanitize handling of none usb controller */ +if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_USB) { +if (controller-model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { +if (usb_other || usb_none) { +virDomainReportError(VIR_ERR_XML_DETAIL, %s, + _(Can't add another USB controller: + USB is disabled for this domain)); +goto error; +} +usb_none = true; +} else { +if (usb_none) { +virDomainReportError(VIR_ERR_XML_DETAIL, %s, + _(Can't add another USB controller: + USB is disabled for this domain)); +goto error; +} +usb_other = true; +} +} + virDomainControllerInsertPreAlloced(def, controller); } VIR_FREE(nodes); @@ -8909,6 +8933,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!input) goto error; +/* Check if USB bus is required */ +if (input-bus == VIR_DOMAIN_INPUT_BUS_USB usb_none) { +virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(Can't add USB input device. + USB bus is disabled)); +goto error; +} /* With QEMU / KVM / Xen graphics, mouse + PS/2 is implicit * with graphics, so don't store it. @@ -9036,6 +9067,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!hostdev) goto error; +if (hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB +usb_none) { +virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(Can't add USB device passthrough: + USB is disabled in this host)); +goto error; +} + def-hostdevs[def-nhostdevs++] = hostdev; } VIR_FREE(nodes); @@ -9105,6 +9144,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!hub) goto error; +if (hub-type == VIR_DOMAIN_HUB_TYPE_USB usb_none) { +virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(Can't add USB hub: + USB is disabled for this domain)); +goto error; +} + def-hubs[def-nhubs++] = hub; } VIR_FREE(nodes); @@ -9121,6 +9167,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!redirdev) goto error; +if (redirdev-bus ==
Re: [libvirt] [PATCH] systemd: start libvirtd after network
On 2012年07月10日 03:41, Jim Fehlig wrote: Domains configured with autostart may fail to start if the host network stack has not been started. E.g. when using bridged networking autostarting a domain can fail with libvirtd[1403]: 2012-06-20 13:23:49.833+: 1485: error : qemuAutostartDomain:177 : Failed to autostart VM 'test': Cannot get interface MTU on 'br0': No such device --- daemon/libvirtd.service.in |1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 33724ee..b7afadf 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -6,6 +6,7 @@ [Unit] Description=Virtualization daemon Before=libvirt-guests.service +After=network.target [Service] EnvironmentFile=-/etc/sysconfig/libvirtd Makes sense from my p.o.v, ACK Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: added description of the vendor_id attribute
--- docs/formatdomain.html.in |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94c555f..b6e0d5d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -779,7 +779,11 @@ in which case an attempt to start a domain requesting an unsupported CPU model will fail. Supported values for codefallback/code attribute are: codeallow/code (this is the default), and -codeforbid/code./dd +codeforbid/code. The optional codevendor_id/code attribute +(span class=sinceSince 0.9.14/span) can be used to set the +vendor id seen by the guest. It must be exactly 12 characters long. +If not set the vendor id of the host is used. Typical possible +values are AuthenticAMD and GenuineIntel./dd dtcodevendor/code/dt ddspan class=sinceSince 0.8.3/span the content of the -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
Am 09.07.2012 19:35, schrieb Corey Bryant: On 07/09/2012 11:46 AM, Kevin Wolf wrote: Am 09.07.2012 17:05, schrieb Corey Bryant: I'm not sure this is an issue with current design. I know things have changed a bit as the email threads evolved, so I'll paste the current design that I am working from. Please let me know if you still see any issues. FD passing: --- New monitor commands enable adding/removing an fd to/from a set. New monitor command query-fdsets enables querying of current monitor fdsets. The set of fds should all refer to the same file, with each fd having different access flags (ie. O_RDWR, O_RDONLY). qemu_open can then dup the fd that has the matching access mode flags. Design points: -- 1. add-fd - fd is passed via SCM rights and qemu adds fd to first unused fdset (e.g. /dev/fdset/1) - add-fd monitor function initializes the monitor inuse flag for the fdset to true - add-fd monitor function initializes the remove flag for the fd to false - add-fd returns fdset number and received fd number (e.g fd=3) to caller 2. drive_add file=/dev/fdset/1 - qemu_open uses the first fd in fdset1 that has access flags matching the qemu_open action flags and has remove flag set to false - qemu_open increments refcount for the fdset - Need to make sure that if a command like 'device-add' fails that refcount is not incremented 3. add-fd fdset=1 - fd is passed via SCM rights - add-fd monitor function adds the received fd to the specified fdset (or fails if fdset doesn't exist) - add-fd monitor function initializes the remove flag for the fd to false - add-fd returns fdset number and received fd number (e.g fd=4) to caller 4. block-commit - qemu_open performs reopen by using the first fd from the fdset that has access flags matching the qemu_open action flags and has remove flag set to false - qemu_open increments refcount for the fdset - Need to make sure that if a command like 'block-commit' fails that refcount is not incremented 5. remove-fd fdset=1 fd=4 - remove-fd monitor function fails if fdset doesn't exist - remove-fd monitor function turns on remove flag for fd=4 What was again the reason why we keep removed fds in the fdset at all? Because if refcount is 0 for the fd set, then the fd could be in use by a block device. So we keep it around until refcount is decremented to zero, at which point it is safe to close. The removed flag would make sense for a fdset after a hypothetical close-fdset call because the fdset needs to be kept around until the last user closes it, but I think removed fds can be deleted immediately. fds in an fd set really need to be kept around until zero block devices reference them. At that point, if '(refcount == 0 (!inuse || remove))' is true, then we'll officially close the fd. Block devices don't reference an fd in the fdset. There are two references in a block device. The first one is obviously the file descriptor they are using; it is a fd dup()ed from an fd in the fdset, but it's now independent of it. The other reference is the file name that is kept in the BlockDriverState, and it always points to /dev/fdset/X, that is, the whole fdset instead of a single fd. What happens if you remove a file descriptor from an fdset that is in use, is that you can't reopen the fdset with the flags of the removed file descriptor any more. Which I believe is exactly the expected behaviour. libvirt would use this to revoke r/w access, for example (and which behaviour you already provide by checking removed in qemu_open). Are there any other use cases where it makes a difference whether a file descriptor is kept in the fdset with removed=1 or whether it's actually removed from the fdset? I think I might have confused remove-fd and close-fdset in earlier emails in this thread, so I hope this isn't inconsistent with what I said before. Ok no problem. 6. qemu_close (need to replace all close calls in block layer with qemu_close) - qemu_close decrements refcount for fdset - qemu_close closes all fds that have (refcount == 0 (!inuse || remove)) - qemu_close frees the fdset if no fds remain in it 7. disconnecting the QMP monitor - monitor disconnect visits all fdsets on monitor and turns off monitor in-use flag for fdset And close all fds with refcount == 0. Yes, this makes sense. It also makes sense to close removed fds with refcount == 0 in the remove-fd function. Basically this will be the same thing we do in qemu_close. We'll close any fds that evaulate the following as true: (refcount == 0 (!inuse || remove)) Yes, whatever condition we'll come up with, but it should be the same and checked in all places where its value might change. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/7] file descriptor passing using pass-fd
Am 09.07.2012 20:40, schrieb Anthony Liguori: On 06/26/2012 04:10 AM, Daniel P. Berrange wrote: On Fri, Jun 22, 2012 at 02:36:07PM -0400, Corey Bryant wrote: libvirt's sVirt security driver provides SELinux MAC isolation for Qemu guest processes and their corresponding image files. In other words, sVirt uses SELinux to prevent a QEMU process from opening files that do not belong to it. sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, and therefore cannot support sVirt isolation. A solution to this problem is to provide fd passing support, where libvirt opens files and passes file descriptors to QEMU. This, along with SELinux policy to prevent QEMU from opening files, can provide image file isolation for NFS files stored on the same NFS mount. This patch series adds the pass-fd QMP monitor command, which allows an fd to be passed via SCM_RIGHTS, and returns the received file descriptor. Support is also added to the block layer to allow QEMU to dup the fd when the filename is of the /dev/fd/X format. This is useful if MAC policy prevents QEMU from opening specific types of files. I was thinking about some of the sources complexity when using FD passing from libvirt and wanted to raise one idea for discussion before we continue. With this proposed series, we have usage akin to: 1. pass_fd FDSET={M} - returns a string /dev/fd/N showing QEMU's view of the FD 2. drive_add file=/dev/fd/N 3. if failure: close_fd /dev/fd/N My problem is that none of this FD passing is transactional. My original patch series did not suffer from this problem. QEMU owned the file descriptor once it received it from libvirt. I don't think the cited problem (QEMU failing an operation if libvirt was down) is really an actual problem since it would be libvirt that would be issuing the command in the first place (so the command would just fail which libvirt would have to assume anyway if it crashed). I really dislike where this thread has headed with /dev/fdset. This has become extremely complex and cumbersome. What exactly is complex about the interface we're going to provide? A long discussion about how to get the details implemented best doesn't mean at all that the result is complex. Perhaps we should reconsider using an RPC for QEMU to request an fd as this solves all the cited problems in a much simpler fashion. NACK. RPC is wrong and no way easier to handle for management. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/7] file descriptor passing using pass-fd
On Mon, Jul 09, 2012 at 04:00:37PM -0300, Luiz Capitulino wrote: On Mon, 09 Jul 2012 13:40:34 -0500 Anthony Liguori aligu...@us.ibm.com wrote: On 06/26/2012 04:10 AM, Daniel P. Berrange wrote: On Fri, Jun 22, 2012 at 02:36:07PM -0400, Corey Bryant wrote: libvirt's sVirt security driver provides SELinux MAC isolation for Qemu guest processes and their corresponding image files. In other words, sVirt uses SELinux to prevent a QEMU process from opening files that do not belong to it. sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, and therefore cannot support sVirt isolation. A solution to this problem is to provide fd passing support, where libvirt opens files and passes file descriptors to QEMU. This, along with SELinux policy to prevent QEMU from opening files, can provide image file isolation for NFS files stored on the same NFS mount. This patch series adds the pass-fd QMP monitor command, which allows an fd to be passed via SCM_RIGHTS, and returns the received file descriptor. Support is also added to the block layer to allow QEMU to dup the fd when the filename is of the /dev/fd/X format. This is useful if MAC policy prevents QEMU from opening specific types of files. I was thinking about some of the sources complexity when using FD passing from libvirt and wanted to raise one idea for discussion before we continue. With this proposed series, we have usage akin to: 1. pass_fd FDSET={M} - returns a string /dev/fd/N showing QEMU's view of the FD 2. drive_add file=/dev/fd/N 3. if failure: close_fd /dev/fd/N My problem is that none of this FD passing is transactional. My original patch series did not suffer from this problem. QEMU owned the file descriptor once it received it from libvirt. I don't think the cited problem (QEMU failing an operation if libvirt was down) is really an actual problem since it would be libvirt that would be issuing the command in the first place (so the command would just fail which libvirt would have to assume anyway if it crashed). I really dislike where this thread has headed with /dev/fdset. This has become extremely complex and cumbersome. I agree, maybe it's time to start over and discuss the original problem again. I must say, I'm not entirely sure of all the problems we're trying to solve anymore. I don't think we've ever clearly stated in this thread what all the requirements/problems are, so I'm finding it hard to see what the optimal solution is. 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 00/13] Support hypervisor-threads-pin in vcpupin.
Hi~ Users can use vcpupin command to bind a vcpu thread to a specific physical cpu. But besides vcpu threads, there are alse some other threads created by qemu (known as hypervisor threads) that could not be explicitly bound to physical cpus. The first 3 patches are from Wen Congyang, which implement Cgroup for differrent hypervisors. The other 10 patches implemented hypervisor threads binding, in two ways: 1) Use sched_setaffinity() function; 2) Use cpuset cgroup. A new xml element is introduced, and vcpupin command is improved, see below. 1. Introduce new xml elements: cputune .. hypervisorpin cpuset='1'/ /cputune 2. Improve vcpupin command to support hypervisor threads binding. For example, vm1 has the following configuration: cputune vcpupin vcpu='1' cpuset='1'/ vcpupin vcpu='0' cpuset='0'/ hypervisorpin cpuset='1'/ /cputune 1) query all threads pining # vcpupin vm1 VCPU: CPU Affinity -- 0: 0 1: 1 Hypervisor: CPU Affinity -- *: 1 2) query hypervisor threads pining only # vcpupin vm1 --hypervisor Hypervisor: CPU Affinity -- *: 1 3) change hypervisor threads pining # vcpupin vm1 --hypervisor 0-1 # vcpupin vm1 --hypervisor Hypervisor: CPU Affinity -- *: 0-1 # taskset -p 397 pid 397's current affinity mask: 3 Note: If users want to pin a vcpu thread to pcpu, --vcpu option could no longer be omitted. Tang Chen (10): Enable cpuset cgroup and synchronous vcpupin info to cgroup. Support hypervisorpin xml parse. Introduce qemuSetupCgroupHypervisorPin and synchronize hypervisorpin info to cgroup. Add qemuProcessSetHypervisorAffinites and set hypervisor threads affinities Introduce virDomainHypervisorPinAdd and virDomainHypervisorPinDel functions Introduce qemudDomainPinHypervisorFlags and qemudDomainGetHypervisorPinInfo in qemu driver. Introduce remoteDomainPinHypervisorFlags and remoteDomainGetHypervisorPinInfo functions in remote driver. Introduce remoteDispatchDomainPinHypervisorFlags and remoteDispatchDomainGetHypervisorPinInfo functions. Introduce virDomainPinHypervisorFlags and virDomainGetHypervisorPinInfo functions. Improve vcpupin to support hypervisorpin dynically. Wen Congyang (3): Introduce the function virCgroupForHypervisor Introduce the function virCgroupMoveTask create a new cgroup and move all hypervisor threads to the new cgroup .gnulib |2 +- daemon/remote.c | 103 + docs/schemas/domaincommon.rng |7 + include/libvirt/libvirt.h.in| 10 + src/conf/domain_conf.c | 173 ++- src/conf/domain_conf.h |7 + src/driver.h| 13 ++ src/libvirt.c | 147 + src/libvirt_private.syms|7 + src/libvirt_public.syms |2 + src/qemu/qemu_cgroup.c | 147 - src/qemu/qemu_cgroup.h |5 + src/qemu/qemu_driver.c | 261 ++- src/qemu/qemu_process.c | 60 +- src/remote/remote_driver.c | 102 + src/remote/remote_protocol.x| 23 +- src/remote_protocol-structs | 24 +++ src/util/cgroup.c | 204 +- src/util/cgroup.h | 15 ++ tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |1 + tests/vcpupin |6 +- tools/virsh.c | 147 - tools/virsh.pod | 16 +- 23 files changed, 1405 insertions(+), 77 deletions(-) -- 1.7.10.2 -- Best Regards, Tang chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/13] Introduce the function virCgroupForHypervisor
Hi~ The sign-off-by problems have been solved, and a new rebased patch set will be sent soon. I am testing it, thanks. :) On 07/03/2012 04:40 AM, Eric Blake wrote: On 06/05/2012 02:13 AM, tangchen wrote: Introduce the function virCgroupForHypervisor() to create sub directory for hypervisor thread(include I/O thread, vhost-net thread) According to your cover letter, this patch was written by Wen, but I don't see a From: listing or Signed-off-by or any other indication that would let 'git am' credit Wen as the author. Instead, it tries to credit you, using only your email alias 'tangchen' instead of your full name (again, by the cover letter, and looking at the current contents of AUTHORS, I assume you prefer 'Tang Chen'). --- src/libvirt_private.syms |1 + src/util/cgroup.c| 42 ++ src/util/cgroup.h|4 3 files changed, 47 insertions(+), 0 deletions(-) /** + * virCgroupForHypervisor: + * + * @driver: group for the domain + * @group: Pointer to returned virCgroupPtr + * + * Returns 0 on success or -errno value on failure. Other than that, the patch looks fine to me, but can you please rebase it to latest libvirt.git and resubmit it so that it gets recorded with correct authorship? -- Best Regards, Tang chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/13] Introduce the function virCgroupForHypervisor
Introduce the function virCgroupForHypervisor() to create sub directory for hypervisor thread(include I/O thread, vhost-net thread) Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- .gnulib |2 +- src/libvirt_private.syms |1 + src/util/cgroup.c| 42 ++ src/util/cgroup.h|4 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index a02ba4b..77cef20 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit a02ba4bf889fee4622db87f185c3d0af84d74ae7 +Subproject commit 77cef2022004c4066e805da09a83b2c77f17bdd3 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6625fc6..8925267 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -68,6 +68,7 @@ virCgroupDenyAllDevices; virCgroupDenyDevicePath; virCgroupForDomain; virCgroupForDriver; +virCgroupForHypervisor; virCgroupForVcpu; virCgroupFree; virCgroupGetBlkioWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 5b32881..1ac8278 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -946,6 +946,48 @@ int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED, #endif /** + * virCgroupForHypervisor: + * + * @driver: group for the domain + * @group: Pointer to returned virCgroupPtr + * + * Returns: 0 on success or -errno on failure + */ +#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R +int virCgroupForHypervisor(virCgroupPtr driver, + virCgroupPtr *group, + int create) +{ +int rc; +char *path; + +if (driver == NULL) +return -EINVAL; + +if (virAsprintf(path, %s/hypervisor, driver-path) 0) +return -ENOMEM; + +rc = virCgroupNew(path, group); +VIR_FREE(path); + +if (rc == 0) { +rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); +if (rc != 0) +virCgroupFree(group); +} + +return rc; +} +#else +int virCgroupForHypervisor(virCgroupPtr driver ATTRIBUTE_UNUSED, + virCgroupPtr *group ATTRIBUTE_UNUSED, + int create ATTRIBUTE_UNUSED) +{ +return -ENXIO; +} + +#endif +/** * virCgroupSetBlkioWeight: * * @group: The cgroup to change io weight for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 05325ae..315ebd6 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -47,6 +47,10 @@ int virCgroupForVcpu(virCgroupPtr driver, virCgroupPtr *group, int create); +int virCgroupForHypervisor(virCgroupPtr driver, + virCgroupPtr *group, + int create); + int virCgroupPathOfController(virCgroupPtr group, int controller, const char *key, -- 1.7.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/13] Enable cpuset cgroup and synchronous vcpupin info to cgroup.
vcpu threads pin are implemented using sched_setaffinity(), but not controlled by cgroup. This patch does the following things: 1) enable cpuset cgroup 2) reflect all the vcpu threads pin info to cgroup Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/libvirt_private.syms |2 ++ src/qemu/qemu_cgroup.c | 44 src/qemu/qemu_cgroup.h |2 ++ src/qemu/qemu_driver.c | 38 ++ src/util/cgroup.c| 35 ++- src/util/cgroup.h|3 +++ 6 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 812cf1d..f6fdc66 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -79,6 +79,7 @@ virCgroupGetCpuShares; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; virCgroupGetCpuacctUsage; +virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetFreezerState; virCgroupGetMemSwapHardLimit; @@ -97,6 +98,7 @@ virCgroupSetBlkioWeight; virCgroupSetCpuCfsPeriod; virCgroupSetCpuCfsQuota; virCgroupSetCpuShares; +virCgroupSetCpusetCpus; virCgroupSetCpusetMems; virCgroupSetFreezerState; virCgroupSetMemSwapHardLimit; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 5f7e8b0..6c811ce 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -480,11 +480,49 @@ cleanup: return -1; } +int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainDefPtr def, + int vcpuid) +{ +int i, rc = 0; +char *new_cpus = NULL; + +if (vcpuid 0 || vcpuid = def-vcpus) { +virReportSystemError(EINVAL, + _(invalid vcpuid: %d), vcpuid); +return -EINVAL; +} + +for (i = 0; i def-cputune.nvcpupin; i++) { +if (vcpuid == def-cputune.vcpupin[i]-vcpuid) { +new_cpus = virDomainCpuSetFormat(def-cputune.vcpupin[i]-cpumask, + VIR_DOMAIN_CPUMASK_LEN); +if (!new_cpus) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(failed to convert cpu mask)); +rc = -1; +goto cleanup; +} +rc = virCgroupSetCpusetCpus(cgroup, new_cpus); +if (rc != 0) { +virReportSystemError(-rc, + %s, + _(Unable to set cpuset.cpus)); +goto cleanup; +} +} +} + +cleanup: +VIR_FREE(new_cpus); +return rc; +} + int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) { virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_vcpu = NULL; qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainDefPtr def = vm-def; int rc; unsigned int i; unsigned long long period = vm-def-cputune.period; @@ -557,6 +595,12 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } } +/* Set vcpupin in cgroup if vcpupin xml is provided */ +if (def-cputune.nvcpupin +qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) +qemuSetupCgroupVcpuPin(cgroup_vcpu, def, i) 0) +goto cleanup; + virCgroupFree(cgroup_vcpu); } diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index cf0d383..91d5632 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -53,6 +53,8 @@ int qemuSetupCgroup(struct qemud_driver *driver, int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); +int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainDefPtr def, + int vcpuid); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3410535..9f795c1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3613,6 +3613,8 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, struct qemud_driver *driver = dom-conn-privateData; virDomainObjPtr vm; virDomainDefPtr persistentDef = NULL; +virCgroupPtr cgroup_dom = NULL; +virCgroupPtr cgroup_vcpu = NULL; int maxcpu, hostcpus; virNodeInfo nodeinfo; int ret = -1; @@ -3667,9 +3669,32 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, if (flags VIR_DOMAIN_AFFECT_LIVE) { if (priv-vcpupids != NULL) { +/* Add config to vm-def first, because cgroup APIs need it. */ +if (virDomainVcpuPinAdd(vm-def, cpumap, maplen, vcpu) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(failed to update or add vcpupin
[libvirt] [PATCH 03/13] create a new cgroup and move all hypervisor threads to the new cgroup
create a new cgroup and move all hypervisor threads to the new cgroup. And then we can do the other things: 1. limit only vcpu usage rather than the whole qemu 2. limit for hypervisor threads(include vhost-net threads) Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- src/qemu/qemu_cgroup.c | 67 --- src/qemu/qemu_cgroup.h |2 ++ src/qemu/qemu_process.c |6 - 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f8f375f..5f7e8b0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -523,11 +523,12 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) { -/* If we does not know VCPU-PID mapping or all vcpu runs in the same +/* If we does not know VCPU-PID mapping or all vcpus run in the same * thread, we cannot control each vcpu. */ -virCgroupFree(cgroup); -return 0; +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(Unable to get vcpus' pids.)); +goto cleanup; } for (i = 0; i priv-nvcpupids; i++) { @@ -564,7 +565,11 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) return 0; cleanup: -virCgroupFree(cgroup_vcpu); +if (cgroup_vcpu) { +virCgroupRemove(cgroup_vcpu); +virCgroupFree(cgroup_vcpu); +} + if (cgroup) { virCgroupRemove(cgroup); virCgroupFree(cgroup); @@ -573,6 +578,60 @@ cleanup: return -1; } +int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, + virDomainObjPtr vm) +{ +virCgroupPtr cgroup = NULL; +virCgroupPtr cgroup_hypervisor = NULL; +int rc, i; + +if (driver-cgroup == NULL) +return 0; /* Not supported, so claim success */ + +rc = virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0); +if (rc != 0) { +virReportSystemError(-rc, + _(Unable to find cgroup for %s), + vm-def-name); +goto cleanup; +} + +rc = virCgroupForHypervisor(cgroup, cgroup_hypervisor, 1); +if (rc 0) { +virReportSystemError(-rc, + _(Unable to create hypervisor cgroup for %s), + vm-def-name); +goto cleanup; +} + +for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) { +rc = virCgroupMoveTask(cgroup, cgroup_hypervisor, i); +if (rc 0) { +virReportSystemError(-rc, + _(Unable to move taks from domain cgroup to + hypervisor cgroup for %s), + vm-def-name); +goto cleanup; +} +} + +virCgroupFree(cgroup_hypervisor); +virCgroupFree(cgroup); +return 0; + +cleanup: +if (cgroup_hypervisor) { +virCgroupRemove(cgroup_hypervisor); +virCgroupFree(cgroup_hypervisor); +} + +if (cgroup) { +virCgroupRemove(cgroup); +virCgroupFree(cgroup); +} + +return rc; +} int qemuRemoveCgroup(struct qemud_driver *driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index c1023b3..cf0d383 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -54,6 +54,8 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); +int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, + virDomainObjPtr vm); int qemuRemoveCgroup(struct qemud_driver *driver, virDomainObjPtr vm, int quiet); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c5140c3..dcd4941 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3740,10 +3740,14 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessDetectVcpuPIDs(driver, vm) 0) goto cleanup; -VIR_DEBUG(Setting cgroup for each VCPU(if required)); +VIR_DEBUG(Setting cgroup for each VCPU (if required)); if (qemuSetupCgroupForVcpu(driver, vm) 0) goto cleanup; +VIR_DEBUG(Setting cgroup for hypervisor (if required)); +if (qemuSetupCgroupForHypervisor(driver, vm) 0) +goto cleanup; + VIR_DEBUG(Setting VCPU affinities); if (qemuProcessSetVcpuAffinites(conn, vm) 0) goto cleanup; -- 1.7.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/13] Support hypervisorpin xml parse.
This patch adds a new xml element hypervisorpin cpuset='1', and also the parser functions, docs, and tests. hypervisorpin means pinning hypervisor threads, and cpuset = '1' means pinning all hypervisor threads to cpu 1. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- docs/schemas/domaincommon.rng |7 ++ src/conf/domain_conf.c | 97 ++- src/conf/domain_conf.h |1 + tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |1 + 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3d205b0..f5cedeb 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -556,6 +556,13 @@ /attribute /element /zeroOrMore + optional +element name=hypervisorpin + attribute name=cpuset +ref name=cpuset/ + /attribute +/element + /optional /element /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3fb90db..376c1b5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7819,6 +7819,51 @@ error: goto cleanup; } +/* Parse the XML definition for hypervisorpin */ +static virDomainVcpuPinDefPtr +virDomainHypervisorPinDefParseXML(const xmlNodePtr node) +{ +virDomainVcpuPinDefPtr def = NULL; +char *tmp = NULL; + +if (VIR_ALLOC(def) 0) { +virReportOOMError(); +return NULL; +} + +def-vcpuid = -1; + +tmp = virXMLPropString(node, cpuset); + +if (tmp) { +char *set = tmp; +int cpumasklen = VIR_DOMAIN_CPUMASK_LEN; + +if (VIR_ALLOC_N(def-cpumask, cpumasklen) 0) { +virReportOOMError(); +goto error; +} + +if (virDomainCpuSetParse(set, 0, def-cpumask, + cpumasklen) 0) +goto error; + +VIR_FREE(tmp); +} else { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, + %s, _(missing cpuset for hypervisor pin)); +goto error; +} + +cleanup: +return def; + +error: +VIR_FREE(tmp); +VIR_FREE(def); +goto cleanup; +} + static int virDomainDefMaybeAddController(virDomainDefPtr def, int type, int idx) @@ -8212,6 +8257,34 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(nodes); +if ((n = virXPathNodeSet(./cputune/hypervisorpin, ctxt, nodes)) 0) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot extract hypervisorpin nodes)); +goto error; +} + +if (n 1) { +virDomainReportError(VIR_ERR_XML_ERROR, %s, + _(only one hypervisorpin is supported)); +VIR_FREE(nodes); +goto error; +} + +if (n VIR_ALLOC(def-cputune.hypervisorpin) 0) { +goto no_memory; +} + +if (n) { +virDomainVcpuPinDefPtr hypervisorpin = NULL; +hypervisorpin = virDomainHypervisorPinDefParseXML(nodes[0]); + +if (!hypervisorpin) +goto error; + +def-cputune.hypervisorpin = hypervisorpin; +} +VIR_FREE(nodes); + /* Extract numatune if exists. */ if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -9216,7 +9289,7 @@ no_memory: virReportOOMError(); /* fallthrough */ - error: +error: VIR_FREE(tmp); VIR_FREE(nodes); virBitmapFree(bootMap); @@ -12784,7 +12857,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, %u/vcpu\n, def-maxvcpus); if (def-cputune.shares || def-cputune.vcpupin || -def-cputune.period || def-cputune.quota) +def-cputune.period || def-cputune.quota || +def-cputune.hypervisorpin) virBufferAddLit(buf, cputune\n); if (def-cputune.shares) @@ -12816,8 +12890,25 @@ virDomainDefFormatInternal(virDomainDefPtr def, } } +if (def-cputune.hypervisorpin) { +virBufferAsprintf(buf, hypervisorpin ); + +char *cpumask = NULL; +cpumask = virDomainCpuSetFormat(def-cputune.hypervisorpin-cpumask, +VIR_DOMAIN_CPUMASK_LEN); +if (cpumask == NULL) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, + %s, _(failed to format cpuset for hypervisor)); +goto cleanup; +} + +virBufferAsprintf(buf, cpuset='%s'/\n, cpumask); +VIR_FREE(cpumask); +} + if (def-cputune.shares || def-cputune.vcpupin || -def-cputune.period || def-cputune.quota) +def-cputune.period || def-cputune.quota || +def-cputune.hypervisorpin)
[libvirt] [PATCH 06/13] Introduce qemuSetupCgroupHypervisorPin and synchronize hypervisorpin info to cgroup.
Introduce qemuSetupCgroupHypervisorPin() function to add hypervisor threads pin info to cpuset cgroup, the same as vcpupin. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/qemu/qemu_cgroup.c | 36 src/qemu/qemu_cgroup.h |1 + 2 files changed, 37 insertions(+) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 6c811ce..cef7901 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -517,6 +517,36 @@ cleanup: return rc; } +int qemuSetupCgroupHypervisorPin(virCgroupPtr cgroup, virDomainDefPtr def) +{ +int rc = 0; +char *new_cpus = NULL; + +if (!def-cputune.hypervisorpin) +return 0; + +new_cpus = virDomainCpuSetFormat(def-cputune.hypervisorpin-cpumask, + VIR_DOMAIN_CPUMASK_LEN); +if (!new_cpus) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(failed to convert cpu mask)); +rc = -1; +goto cleanup; +} + +rc = virCgroupSetCpusetCpus(cgroup, new_cpus); +if (rc 0) { +virReportSystemError(-rc, + %s, + _(Unable to set cpuset.cpus)); +goto cleanup; +} + +cleanup: +VIR_FREE(new_cpus); +return rc; +} + int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) { virCgroupPtr cgroup = NULL; @@ -627,6 +657,7 @@ int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, { virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_hypervisor = NULL; +virDomainDefPtr def = vm-def; int rc, i; if (driver-cgroup == NULL) @@ -659,6 +690,11 @@ int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, } } +if (def-cputune.hypervisorpin +qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) +qemuSetupCgroupHypervisorPin(cgroup_hypervisor, def) 0) +goto cleanup; + virCgroupFree(cgroup_hypervisor); virCgroupFree(cgroup); return 0; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 91d5632..12444c3 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -55,6 +55,7 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, long long quota); int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainDefPtr def, int vcpuid); +int qemuSetupCgroupHypervisorPin(virCgroupPtr cgroup, virDomainDefPtr def); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, virDomainObjPtr vm); -- 1.7.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/13] Add qemuProcessSetHypervisorAffinites and set hypervisor threads affinities
Hypervisor threads should also be pinned by sched_setaffinity(), just the same as vcpu threads. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/qemu/qemu_process.c | 54 +++ 1 file changed, 54 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index dcd4941..373e212 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1974,6 +1974,56 @@ cleanup: return ret; } +/* Set CPU affinities for hypervisor threads if hypervisorpin xml provided. */ +static int +qemuProcessSetHypervisorAffinites(virConnectPtr conn, + virDomainObjPtr vm) +{ +virDomainDefPtr def = vm-def; +pid_t pid = vm-pid; +unsigned char *cpumask = NULL; +unsigned char *cpumap = NULL; +virNodeInfo nodeinfo; +int cpumaplen, hostcpus, maxcpu, i; +int ret = -1; + +if (virNodeGetInfo(conn, nodeinfo) != 0) +return -1; + +if (!def-cputune.hypervisorpin) +return 0; + +hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); +cpumaplen = VIR_CPU_MAPLEN(hostcpus); +maxcpu = cpumaplen * 8; + +if (maxcpu hostcpus) +maxcpu = hostcpus; + +if (VIR_ALLOC_N(cpumap, cpumaplen) 0) { +virReportOOMError(); +return -1; +} + +cpumask = (unsigned char *)def-cputune.hypervisorpin-cpumask; +for(i = 0; i VIR_DOMAIN_CPUMASK_LEN; i++) { +if (cpumask[i]) +VIR_USE_CPU(cpumap, i); +} + +if (virProcessInfoSetAffinity(pid, + cpumap, + cpumaplen, + maxcpu) 0) { +goto cleanup; +} + +ret = 0; +cleanup: +VIR_FREE(cpumap); +return ret; +} + static int qemuProcessInitPasswords(virConnectPtr conn, struct qemud_driver *driver, @@ -3752,6 +3802,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessSetVcpuAffinites(conn, vm) 0) goto cleanup; +VIR_DEBUG(Setting hypervisor threads affinities); +if (qemuProcessSetHypervisorAffinites(conn, vm) 0) +goto cleanup; + VIR_DEBUG(Setting any required VM passwords); if (qemuProcessInitPasswords(conn, driver, vm) 0) goto cleanup; -- 1.7.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/13] Introduce virDomainHypervisorPinAdd and virDomainHypervisorPinDel functions
Introduce 2 APIs to support hypervisor threads pin. 1) virDomainHypervisorPinAdd: setup hypervisor threads pin with a given cpumap string. 2) virDomainHypervisorPinDel: remove all hypervisor threads pin. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/conf/domain_conf.c | 76 ++ src/conf/domain_conf.h |6 src/libvirt_private.syms |2 ++ 3 files changed, 84 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 376c1b5..9bd144a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10947,6 +10947,82 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) return 0; } +int +virDomainHypervisorPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen) +{ +virDomainVcpuPinDefPtr hypervisorpin = NULL; +char *cpumask = NULL; +int i; + +if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) 0) { +virReportOOMError(); +goto cleanup; +} + +/* Reset cpumask to all 0s. */ +for (i = 0; i VIR_DOMAIN_CPUMASK_LEN; i++) +cpumask[i] = 0; + +/* Convert bitmap (cpumap) to cpumask, which is byte map. */ +for (i = 0; i maplen; i++) { +int cur; + +for (cur = 0; cur 8; cur++) { +if (cpumap[i] (1 cur)) +cpumask[i * 8 + cur] = 1; +} +} + +if (!def-cputune.hypervisorpin) { +/* No hypervisorpin exists yet. */ +if (VIR_ALLOC(hypervisorpin) 0) { +virReportOOMError(); +goto cleanup; +} + +hypervisorpin-vcpuid = -1; +hypervisorpin-cpumask = cpumask; +def-cputune.hypervisorpin = hypervisorpin; +} else { +/* Since there is only 1 hypervisorpin for each vm, + * juest replace the old one. + */ +VIR_FREE(def-cputune.hypervisorpin-cpumask); +def-cputune.hypervisorpin-cpumask = cpumask; +} + +return 0; + +cleanup: +if (cpumask) +VIR_FREE(cpumask); +return -1; +} + +int +virDomainHypervisorPinDel(virDomainDefPtr def) +{ +virDomainVcpuPinDefPtr hypervisorpin = NULL; + +/* No hypervisorpin exists yet */ +if (!def-cputune.hypervisorpin) { +return 0; +} + +hypervisorpin = def-cputune.hypervisorpin; + +VIR_FREE(hypervisorpin-cpumask); +VIR_FREE(hypervisorpin); +def-cputune.hypervisorpin = NULL; + +if (def-cputune.hypervisorpin) +return -1; + +return 0; +} + static int virDomainLifecycleDefFormat(virBufferPtr buf, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3768b82..bea8026 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1984,6 +1984,12 @@ int virDomainVcpuPinAdd(virDomainDefPtr def, int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); +int virDomainHypervisorPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen); + +int virDomainHypervisorPinDel(virDomainDefPtr def); + int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, bool allow_ambiguous); const char *virDomainDiskPathByName(virDomainDefPtr, const char *name); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f6fdc66..e246107 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -488,6 +488,8 @@ virDomainTimerTrackTypeFromString; virDomainTimerTrackTypeToString; virDomainVcpuPinAdd; virDomainVcpuPinDel; +virDomainHypervisorPinAdd; +virDomainHypervisorPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; virDomainVideoDefFree; -- 1.7.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/13] Introduce remoteDispatchDomainPinHypervisorFlags and remoteDispatchDomainGetHypervisorPinInfo functions.
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- daemon/remote.c | 103 +++ 1 file changed, 103 insertions(+) diff --git a/daemon/remote.c b/daemon/remote.c index 095d854..15dfa9b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1509,6 +1509,109 @@ no_memory: } static int +remoteDispatchDomainPinHypervisorFlags(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_pin_hypervisor_flags_args *args) +{ +int rv = -1; +virDomainPtr dom = NULL; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if (virDomainPinHypervisorFlags(dom, +(unsigned char *) args-cpumap.cpumap_val, +args-cpumap.cpumap_len, +args-flags) 0) +goto cleanup; + +rv = 0; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + + +static int +remoteDispatchDomainGetHypervisorPinInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_hypervisor_pin_info_args *args, + remote_domain_get_hypervisor_pin_info_ret *ret) +{ +virDomainPtr dom = NULL; +unsigned char *cpumaps = NULL; +int num; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +/* There is only one cpumap struct for all hypervisor threads */ +if (args-ncpumaps != 1) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(ncpumaps != 1)); +goto cleanup; +} + +if (INT_MULTIPLY_OVERFLOW(args-ncpumaps, args-maplen) || +args-ncpumaps * args-maplen REMOTE_CPUMAPS_MAX) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(maxinfo * maplen REMOTE_CPUMAPS_MAX)); +goto cleanup; +} + +/* Allocate buffers to take the results */ +if (args-maplen 0 +VIR_ALLOC_N(cpumaps, args-maplen) 0) +goto no_memory; + +if ((num = virDomainGetHypervisorPinInfo(dom, + cpumaps, + args-maplen, + args-flags)) 0) +goto cleanup; + +ret-num = num; +ret-cpumaps.cpumaps_len = args-maplen; +ret-cpumaps.cpumaps_val = (char *) cpumaps; +cpumaps = NULL; + +rv = 0; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +VIR_FREE(cpumaps); +if (dom) +virDomainFree(dom); +return rv; + +no_memory: +virReportOOMError(); +goto cleanup; +} + +static int remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, -- 1.7.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/13] Introduce the function virCgroupMoveTask
Introduce a new API to move tasks of one controller from a cgroup to another cgroup Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- src/libvirt_private.syms |2 + src/util/cgroup.c| 127 ++ src/util/cgroup.h|8 +++ 3 files changed, 137 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8925267..812cf1d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -60,6 +60,7 @@ virCapabilitiesSetMacPrefix; # cgroup.h virCgroupAddTask; +virCgroupAddTaskController; virCgroupAllowDeviceMajor; virCgroupAllowDevicePath; virCgroupControllerTypeFromString; @@ -88,6 +89,7 @@ virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; virCgroupMounted; +virCgroupMoveTask; virCgroupPathOfController; virCgroupRemove; virCgroupSetBlkioDeviceWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 1ac8278..3e9ba4e 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -791,6 +791,133 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) return rc; } +/** + * virCgroupAddTaskController: + * + * @group: The cgroup to add a task to + * @pid: The pid of the task to add + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -errno on failure + */ +int virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) +{ +int rc = 0; + +if (controller VIR_CGROUP_CONTROLLER_CPU || +controller VIR_CGROUP_CONTROLLER_BLKIO) +return -EINVAL; + +if (!group-controllers[controller].mountPoint) +return -EINVAL; + +return virCgroupSetValueU64(group, controller, tasks, +(unsigned long long)pid); +} + +static int virCgroupAddTaskStr(virCgroupPtr group, const char *pidstr) +{ +unsigned long long value; + +if (virStrToLong_ull(pidstr, NULL, 10, value) 0) +return -EINVAL; + +return virCgroupAddTask(group, value); +} + +static int virCgroupAddTaskStrController(virCgroupPtr group, +const char *pidstr, +int controller) +{ +char *str = NULL, *cur = NULL, *next = NULL; +unsigned long long pid = 0; +int len = 0, rc = 0; + +len = strlen(pidstr); +VIR_ALLOC_N(str, len); +if (str == NULL) { +VIR_ERROR(_(No more memory.)); +return -1; +} +rc = strcpy(str, pidstr); +if (rc != 0) +return rc; + +cur = str; +while ((next = strchr(cur, '\n')) != NULL) { +*next = '\0'; +rc = virStrToLong_ull(cur, NULL, 10, pid); +if (rc != 0) +goto cleanup; +cur = next + 1; + +rc = virCgroupAddTaskController(group, pid, controller); +if (rc != 0) +goto cleanup; +} +if (cur != '\0') { +rc = virStrToLong_ull(cur, NULL, 10, pid); +if (rc != 0) +goto cleanup; +rc = virCgroupAddTaskController(group, pid, controller); +if (rc != 0) +goto cleanup; +} + +cleanup: +VIR_FREE(str); +return rc; +} + +/** + * virCgroupMoveTask: + * + * @src_group: The source cgroup where all tasks are removed from + * @dest_group: The destination where all tasks are added to + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -errno on failure + */ +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group, + int controller) +{ +int rc = 0, err = 0; +char *content = NULL; + +if (controller VIR_CGROUP_CONTROLLER_CPU || +controller VIR_CGROUP_CONTROLLER_BLKIO) +return -EINVAL; + +if (!src_group-controllers[controller].mountPoint || +!dest_group-controllers[controller].mountPoint) +return -EINVAL; + +rc = virCgroupGetValueStr(src_group, controller, tasks, content); +if (rc != 0) +return rc; + +rc = virCgroupAddTaskStrController(dest_group, content, controller); +if (rc != 0) +goto cleanup; + +VIR_FREE(content); + +return 0; + +cleanup: +/* + * We don't need to recover dest_cgroup because cgroup will make sure + * that one task only resides in one cgroup of the same controller. + */ +err = virCgroupAddTaskStrController(src_group, content, controller); +if (err != 0) +VIR_ERROR(_(Cannot recover cgroup %s from %s), + src_group-controllers[controller].mountPoint, + dest_group-controllers[controller].mountPoint); +VIR_FREE(content); + +return rc; +} /** * virCgroupForDriver: diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 315ebd6..f14c167 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -58,6 +58,14 @@ int virCgroupPathOfController(virCgroupPtr group, int virCgroupAddTask(virCgroupPtr group, pid_t pid); +int virCgroupAddTaskController(virCgroupPtr group, +
[libvirt] [PATCH 09/13] Introduce qemudDomainPinHypervisorFlags and qemudDomainGetHypervisorPinInfo in qemu driver.
Introduce 2 APIs to support hypervisor threads pin in qemu driver. 1) qemudDomainPinHypervisorFlags: setup hypervisor threads pin info. 2) qemudDomainGetHypervisorPinInfo: get all hypervisor threads pin info. They are similar to qemudDomainPinVcpuFlags and qemudDomainGetVcpuPinInfo. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/driver.h | 13 +++ src/qemu/qemu_driver.c | 223 2 files changed, 236 insertions(+) diff --git a/src/driver.h b/src/driver.h index b3c1740..31db44d 100644 --- a/src/driver.h +++ b/src/driver.h @@ -302,6 +302,17 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainPinHypervisorFlags) (virDomainPtr domain, + unsigned char *cpumap, + int maplen, + unsigned int flags); +typedef int +(*virDrvDomainGetHypervisorPinInfo) (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags); + +typedef int (*virDrvDomainGetVcpus) (virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, @@ -931,6 +942,8 @@ struct _virDriver { virDrvDomainPinVcpu domainPinVcpu; virDrvDomainPinVcpuFlagsdomainPinVcpuFlags; virDrvDomainGetVcpuPinInfo domainGetVcpuPinInfo; +virDrvDomainPinHypervisorFlagsdomainPinHypervisorFlags; +virDrvDomainGetHypervisorPinInfo domainGetHypervisorPinInfo; virDrvDomainGetVcpusdomainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabeldomainGetSecurityLabel; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f795c1..3a0ce2f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3845,6 +3845,227 @@ cleanup: } static int +qemudDomainPinHypervisorFlags(virDomainPtr dom, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm; +virCgroupPtr cgroup_dom = NULL; +virCgroupPtr cgroup_hypervisor = NULL; +pid_t pid; +virDomainDefPtr persistentDef = NULL; +int maxcpu, hostcpus; +virNodeInfo nodeinfo; +int ret = -1; +qemuDomainObjPrivatePtr priv; +bool canResetting = true; +int pcpu; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +qemuDriverLock(driver); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +qemuDriverUnlock(driver); + +if (!vm) { +char uuidstr[VIR_UUID_STRING_BUFLEN]; +virUUIDFormat(dom-uuid, uuidstr); +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +if (virDomainLiveConfigHelperMethod(driver-caps, vm, flags, +persistentDef) 0) +goto cleanup; + +priv = vm-privateData; + +if (nodeGetInfo(dom-conn, nodeinfo) 0) +goto cleanup; +hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); +maxcpu = maplen * 8; +if (maxcpu hostcpus) +maxcpu = hostcpus; +/* pinning to all physical cpus means resetting, + * so check if we can reset setting. + */ +for (pcpu = 0; pcpu hostcpus; pcpu++) { +if ((cpumap[pcpu/8] (1 (pcpu % 8))) == 0) { +canResetting = false; +break; +} +} + +pid = vm-pid; + +if (flags VIR_DOMAIN_AFFECT_LIVE) { + +if (priv-vcpupids != NULL) { +if (virDomainHypervisorPinAdd(vm-def, cpumap, maplen) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(failed to update or add hypervisorpin xml + of a running domain)); +goto cleanup; +} + +if (qemuCgroupControllerActive(driver, + VIR_CGROUP_CONTROLLER_CPUSET)) { +/* + * Configure the corresponding cpuset cgroup. + * If no cgroup for domain or hypervisor exists, do nothing. + */ +if (virCgroupForDomain(driver-cgroup, vm-def-name, + cgroup_dom, 0) == 0) { +if (virCgroupForHypervisor(cgroup_dom, cgroup_hypervisor, 0) == 0) { +if (qemuSetupCgroupHypervisorPin(cgroup_hypervisor, vm-def) 0) { +
[libvirt] [PATCH 10/13] Introduce remoteDomainPinHypervisorFlags and remoteDomainGetHypervisorPinInfo functions in remote driver.
Introduce 2 APIs to support hypervisor threads in remote driver. 1) remoteDomainPinHypervisorFlags: call driver api, such as qemudDomainPinHypervisorFlags. 2) remoteDomainGetHypervisorPinInfo: call driver api, such as qemudDomainGetHypervisorPinInfo. They are similar to remoteDomainPinVcpuFlags and remoteDomainGetVcpuPinInfo. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/remote/remote_driver.c | 102 ++ src/remote/remote_protocol.x | 23 +- src/remote_protocol-structs | 24 ++ 3 files changed, 148 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index eac50e6..7fd128b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1806,6 +1806,106 @@ done: } static int +remoteDomainPinHypervisorFlags (virDomainPtr dom, +unsigned char *cpumap, +int cpumaplen, +unsigned int flags) +{ +int rv = -1; +struct private_data *priv = dom-conn-privateData; +remote_domain_pin_hypervisor_flags_args args; + +remoteDriverLock(priv); + +if (cpumaplen REMOTE_CPUMAP_MAX) { +remoteError(VIR_ERR_RPC, +_(%s length greater than maximum: %d %d), +cpumap, (int)cpumaplen, REMOTE_CPUMAP_MAX); +goto done; +} + +make_nonnull_domain(args.dom, dom); +args.vcpu = -1; +args.cpumap.cpumap_val = (char *)cpumap; +args.cpumap.cpumap_len = cpumaplen; +args.flags = flags; + +if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_PIN_HYPERVISOR_FLAGS, + (xdrproc_t) xdr_remote_domain_pin_hypervisor_flags_args, + (char *) args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) { +goto done; +} + +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + + +static int +remoteDomainGetHypervisorPinInfo (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ +int rv = -1; +int i; +remote_domain_get_hypervisor_pin_info_args args; +remote_domain_get_hypervisor_pin_info_ret ret; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv); + +/* There is only one cpumap for all hypervisor threads */ +if (INT_MULTIPLY_OVERFLOW(1, maplen) || +maplen REMOTE_CPUMAPS_MAX) { +remoteError(VIR_ERR_RPC, +_(vCPU map buffer length exceeds maximum: %d %d), +maplen, REMOTE_CPUMAPS_MAX); +goto done; +} + +make_nonnull_domain(args.dom, domain); +args.ncpumaps = 1; +args.maplen = maplen; +args.flags = flags; + +memset(ret, 0, sizeof ret); + +if (call (domain-conn, priv, 0, REMOTE_PROC_DOMAIN_GET_HYPERVISOR_PIN_INFO, + (xdrproc_t) xdr_remote_domain_get_hypervisor_pin_info_args, + (char *) args, + (xdrproc_t) xdr_remote_domain_get_hypervisor_pin_info_ret, + (char *) ret) == -1) +goto done; + +if (ret.cpumaps.cpumaps_len maplen) { +remoteError(VIR_ERR_RPC, +_(host reports map buffer length exceeds maximum: %d %d), +ret.cpumaps.cpumaps_len, maplen); +goto cleanup; +} + +memset(cpumaps, 0, maplen); + +for (i = 0; i ret.cpumaps.cpumaps_len; ++i) +cpumaps[i] = ret.cpumaps.cpumaps_val[i]; + +rv = ret.num; + +cleanup: +xdr_free ((xdrproc_t) xdr_remote_domain_get_hypervisor_pin_info_ret, + (char *) ret); + +done: +remoteDriverUnlock(priv); +return rv; +} + +static int remoteDomainGetVcpus (virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, @@ -5192,6 +5292,8 @@ static virDriver remote_driver = { .domainPinVcpu = remoteDomainPinVcpu, /* 0.3.0 */ .domainPinVcpuFlags = remoteDomainPinVcpuFlags, /* 0.9.3 */ .domainGetVcpuPinInfo = remoteDomainGetVcpuPinInfo, /* 0.9.3 */ +.domainPinHypervisorFlags = remoteDomainPinHypervisorFlags, /* 0.9.13 */ +.domainGetHypervisorPinInfo = remoteDomainGetHypervisorPinInfo, /* 0.9.13 */ .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1da9f3e..820553c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1054,6 +1054,25 @@ struct remote_domain_get_vcpu_pin_info_ret { int num; }; +struct remote_domain_pin_hypervisor_flags_args { +remote_nonnull_domain dom; +unsigned int vcpu; +opaque cpumapREMOTE_CPUMAP_MAX; /* (unsigned char *) */ +unsigned int flags; +}; + +struct
Re: [libvirt] [PATCH] [libvirt-java] Fix typo in Domain.java.
On 2012年07月06日 21:38, Claudio Bley wrote: Hi. Just a small typo fix: --- diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index fe9f3b0..71c6397 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -678,7 +678,7 @@ public class Domain { * @return 0 in case of success, and -1 in case of error * @throws LibvirtException */ -public int managedSaveRemote() throws LibvirtException { +public int managedSaveRemove() throws LibvirtException { int returnValue = libvirt.virDomainManagedSaveRemove(VDP, 0); processError(); return returnValue; --- ACK and pushed. AUTHORS is updated with Claudio Bley cb...@av-test.de, please let us known if you have prefered name. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/13] Introduce virDomainPinHypervisorFlags and virDomainGetHypervisorPinInfo functions.
Introduce 2 APIs for client to use. 1) virDomainPinHypervisorFlags: call remote driver api, such as remoteDomainPinHypervisorFlags. 2) virDomainGetHypervisorPinInfo: call remote driver api, such as remoteDomainGetHypervisorPinInfo. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- include/libvirt/libvirt.h.in | 10 +++ src/libvirt.c| 147 ++ src/libvirt_public.syms |2 + 3 files changed, 159 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6e8d5dd..93e9334 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1861,6 +1861,16 @@ int virDomainGetVcpuPinInfo (virDomainPtr domain, int maplen, unsigned int flags); +int virDomainPinHypervisorFlags (virDomainPtr domain, + unsigned char *cpumap, + int maplen, + unsigned int flags); + +int virDomainGetHypervisorPinInfo (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags); + /** * VIR_USE_CPU: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT) diff --git a/src/libvirt.c b/src/libvirt.c index db6ba15..d2f3c65 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8841,6 +8841,153 @@ error: } /** + * virDomainPinHypervisorFlags: + * @domain: pointer to domain object, or NULL for Domain0 + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * Each bit set to 1 means that corresponding CPU is usable. + * Bytes are stored in little-endian order: CPU0-7, 8-15... + * In each byte, lowest CPU number is least significant bit. + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in + * underlying virtualization system (Xen...). + * If maplen size, missing bytes are set to zero. + * If maplen size, failure code is returned. + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically change the real CPUs which can be allocated to all hypervisor + * threads. This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * Not all hypervisors can support all flag combinations. + * + * See also virDomainGetHypervisorPinInfo for querying this information. + * + * Returns 0 in case of success, -1 in case of failure. + * + */ +int +virDomainPinHypervisorFlags(virDomainPtr domain, unsigned char *cpumap, +int maplen, unsigned int flags) +{ +virConnectPtr conn; + +VIR_DOMAIN_DEBUG(domain, cpumap=%p, maplen=%d, flags=%x, + cpumap, maplen, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN(domain)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} + +if (domain-conn-flags VIR_CONNECT_RO) { +virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if ((cpumap == NULL) || (maplen 1)) { +virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); +goto error; +} + +conn = domain-conn; + +if (conn-driver-domainPinHypervisorFlags) { +int ret; +ret = conn-driver-domainPinHypervisorFlags (domain, cpumap, maplen, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(domain-conn); +return -1; +} + +/** + * virDomainGetHypervisorPinInfo: + * @domain: pointer to domain object, or NULL for Domain0 + * @cpumap: pointer to a bit map of real CPUs for all hypervisor threads of + * this domain (in 8-bit bytes) (OUT) + * There is only one cpumap for all hypervisor threads. + * Must not be NULL. + * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: bitwise-OR of virDomainModificationImpact + * Must not be
[libvirt] [PATCH 13/13] Improve vcpupin to support hypervisorpin dynically.
Modify vcpupin command to support hypervisor threads pin. 1) add --hypervisor option to get hypervisor threads info. 2) add --hypervisor cpuset to set hypervisor threads to specified cpuset. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- tests/vcpupin |6 +-- tools/virsh.c | 147 --- tools/virsh.pod | 16 +++--- 3 files changed, 111 insertions(+), 58 deletions(-) diff --git a/tests/vcpupin b/tests/vcpupin index 5952862..ffd16fa 100755 --- a/tests/vcpupin +++ b/tests/vcpupin @@ -30,16 +30,16 @@ fi fail=0 # Invalid syntax. -$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 out 21 +$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a --vcpu 0,1 out 21 test $? = 1 || fail=1 cat \EOF exp || fail=1 -error: vcpupin: Invalid or missing vCPU number. +error: vcpupin: Invalid or missing vCPU number, or missing --hypervisor option. EOF compare exp out || fail=1 # An out-of-range vCPU number deserves a diagnostic, too. -$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 0,1 out 21 +$abs_top_builddir/tools/virsh --connect test:///default vcpupin test --vcpu 100 0,1 out 21 test $? = 1 || fail=1 cat \EOF exp || fail=1 error: vcpupin: Invalid vCPU number. diff --git a/tools/virsh.c b/tools/virsh.c index 85b1185..6d59897 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5463,14 +5463,15 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) * vcpupin command */ static const vshCmdInfo info_vcpupin[] = { -{help, N_(control or query domain vcpu affinity)}, -{desc, N_(Pin domain VCPUs to host physical CPUs.)}, +{help, N_(control or query domain vcpu and hypervisor threads affinities)}, +{desc, N_(Pin domain VCPUs or hypervisor threads to host physical CPUs.)}, {NULL, NULL} }; static const vshCmdOptDef opts_vcpupin[] = { {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, -{vcpu, VSH_OT_INT, 0, N_(vcpu number)}, +{vcpu, VSH_OT_INT, VSH_OFLAG_REQ_OPT, N_(vcpu number)}, +{hypervisor, VSH_OT_BOOL, VSH_OFLAG_REQ_OPT, N_(pin hypervisor threads)}, {cpulist, VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, N_(host cpu number(s) to set, or omit option to query)}, {config, VSH_OT_BOOL, 0, N_(affect next boot)}, @@ -5479,6 +5480,45 @@ static const vshCmdOptDef opts_vcpupin[] = { {NULL, 0, 0, NULL} }; +/* + * Helper function to print vcpupin and hypervisorpin info. + */ +static bool +printPinInfo(unsigned char *cpumaps, size_t cpumaplen, + int maxcpu, int vcpuindex) +{ +int cpu, lastcpu; +bool bit, lastbit, isInvert; + +if (!cpumaps || cpumaplen = 0 || maxcpu = 0 || vcpuindex 0) { +return false; +} + +bit = lastbit = isInvert = false; +lastcpu = -1; + +for (cpu = 0; cpu maxcpu; cpu++) { +bit = VIR_CPU_USABLE(cpumaps, cpumaplen, vcpuindex, cpu); + +isInvert = (bit ^ lastbit); +if (bit isInvert) { +if (lastcpu == -1) +vshPrint(ctl, %d, cpu); +else +vshPrint(ctl, ,%d, cpu); +lastcpu = cpu; +} +if (!bit isInvert lastcpu != cpu - 1) +vshPrint(ctl, -%d, cpu - 1); +lastbit = bit; +} +if (bit !isInvert) { +vshPrint(ctl, -%d, maxcpu - 1); +} + +return true; +} + static bool cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) { @@ -5491,13 +5531,13 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) unsigned char *cpumap = NULL; unsigned char *cpumaps = NULL; size_t cpumaplen; -bool bit, lastbit, isInvert; -int i, cpu, lastcpu, maxcpu, ncpus; +int i, cpu, lastcpu, maxcpu, ncpus, nhyper; bool unuse = false; const char *cur; bool config = vshCommandOptBool(cmd, config); bool live = vshCommandOptBool(cmd, live); bool current = vshCommandOptBool(cmd, current); +bool hypervisor = vshCommandOptBool(cmd, hypervisor); bool query = false; /* Query mode if no cpulist */ unsigned int flags = 0; @@ -5532,8 +5572,18 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) /* In query mode, vcpu is optional */ if (vshCommandOptInt(cmd, vcpu, vcpu) !query) { -vshError(ctl, %s, - _(vcpupin: Invalid or missing vCPU number.)); +if (!hypervisor) { +vshError(ctl, %s, + _(vcpupin: Invalid or missing vCPU number, + or missing --hypervisor option.)); +virDomainFree(dom); +return false; +} +} + +if (hypervisor vcpu != -1) { +vshError(ctl, %s, _(vcpupin: --hypervisor must be specified + exclusively to --vcpu.)); virDomainFree(dom); return false; } @@ -5565,47 +5615,45 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (flags == -1) flags =
Re: [libvirt] [PATCH] docs: added description of the vendor_id attribute
On 10.07.2012 10:01, Hendrik Schwartke wrote: --- docs/formatdomain.html.in |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94c555f..b6e0d5d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -779,7 +779,11 @@ in which case an attempt to start a domain requesting an unsupported CPU model will fail. Supported values for codefallback/code attribute are: codeallow/code (this is the default), and -codeforbid/code./dd +codeforbid/code. The optional codevendor_id/code attribute +(span class=sinceSince 0.9.14/span) can be used to set the +vendor id seen by the guest. It must be exactly 12 characters long. +If not set the vendor id of the host is used. Typical possible +values are AuthenticAMD and GenuineIntel./dd dtcodevendor/code/dt ddspan class=sinceSince 0.8.3/span the content of the ACKed and pushed. Thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/13] Introduce the function virCgroupMoveTask
hi~ +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) +{ +int rc = 0; +int i; +char *content, *value, *next; + +for (i = 0 ; i VIR_CGROUP_CONTROLLER_LAST ; i++) { +/* Skip over controllers not mounted */ +if (!src_group-controllers[i].mountPoint || +!dest_group-controllers[i].mountPoint) +continue; Should we insist that src_group and dest_group have the same set of mounted controllers? I'm worried that if we call this function, but the set of mounted controllers differs between the two sets, then we end up moving processes between some controllers and stranding them in the source for the remaining controllers. True. So I change it to move tasks under one controller, and leave all the other controller unmodified. You can see it in my new patch. :) As you know, different cgroups are independent to each other. So I think operate on only one controller will make sense. +*next = '\0'; +if ((rc = virCgrouAddTaskStr(dest_group, value) 0)) +goto cleanup; +value = next + 1; +} +if (*value != '\0') { +if ((rc = virCgrouAddTaskStr(dest_group, value) 0)) Does it make sense to parse all the string into integers, just to format the integers back into strings? Or would it be simpler to just cat the contents of the 'tasks' file from the source into the destination without bothering to interpret the date in transit? I have tried this. But it seems that tasks file includes \n, so it won't work. +goto cleanup; +} + +VIR_FREE(content); +} + +return 0; + +cleanup: +virCgroupMoveTask(dest_group, src_group); Is this cleanup always correct, or is it only correct if 'dest_group' started life empty? Should we at least log a warning message if a move was partially attempted and then reverted, particularly if the reversion attempt failed? The cleanup way is also changed, please refer to my new patches. Thanks. :) -- Best Regards, Tang chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Use current uid and gid if they are passed as -1 for virDirCreate
ping? On 2012年06月21日 15:11, Osier Yang wrote: All the callers of virDirCreate are updated incidentally. --- src/storage/storage_backend_fs.c | 23 ++- src/util/util.c |6 ++ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bde4528..233e001 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -789,17 +789,10 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, /* Now create the final dir in the path with the uid/gid/mode * requested in the config. If the dir already exists, just set * the perms. */ -uid_t uid; -gid_t gid; - -uid = (pool-def-target.perms.uid == (uid_t) -1) -? getuid() : pool-def-target.perms.uid; -gid = (pool-def-target.perms.gid == (gid_t) -1) -? getgid() : pool-def-target.perms.gid; - if ((err = virDirCreate(pool-def-target.path, pool-def-target.perms.mode, -uid, gid, +pool-def-target.perms.uid, +pool-def-target.perms.gid, VIR_DIR_CREATE_FORCE_PERMS | VIR_DIR_CREATE_ALLOW_EXIST | (pool-def-type == VIR_STORAGE_POOL_NETFS @@ -811,9 +804,9 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, /* Reflect the actual uid and gid to the config. */ if (pool-def-target.perms.uid == (uid_t) -1) -pool-def-target.perms.uid = uid; +pool-def-target.perms.uid = getuid(); if (pool-def-target.perms.gid == (gid_t) -1) -pool-def-target.perms.gid = gid; +pool-def-target.perms.gid = getgid(); if (flags != 0) { ret = virStorageBackendMakeFileSystem(pool, flags); @@ -1053,13 +1046,9 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } -uid_t uid = (vol-target.perms.uid == -1) -? getuid() : vol-target.perms.uid; -gid_t gid = (vol-target.perms.gid == -1) -? getgid() : vol-target.perms.gid; - if ((err = virDirCreate(vol-target.path, vol-target.perms.mode, -uid, gid, +vol-target.perms.uid, +vol-target.perms.gid, VIR_DIR_CREATE_FORCE_PERMS | (pool-def-type == VIR_STORAGE_POOL_NETFS ? VIR_DIR_CREATE_AS_UID : 0))) 0) { diff --git a/src/util/util.c b/src/util/util.c index ce98d20..aec5512 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1123,6 +1123,12 @@ int virDirCreate(const char *path, mode_t mode, int waitret; int status, ret = 0; +/* allow using -1 to mean current value */ +if (uid == (uid_t) -1) +uid = getuid(); +if (gid == (gid_t) -1) +gid = getgid(); + if ((!(flags VIR_DIR_CREATE_AS_UID)) || (getuid() != 0) || ((uid == 0) (gid == 0)) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] conf: Format numatune XML correctly while placement is none
ping? On 2012年06月25日 12:28, Osier Yang wrote: setNumaParameters tunes the numa setting using cgroup, it's another entry except libnuma/numad for numa tuning. And it doesn't set the placement, and further more, the formating codes doesn't take this into consideration. How to reproduce: conn = libvirt.open(None) dom = conn.lookupByName('linux') param = {'numa_nodeset': '0', 'numa_mode': 1} dom.setNumaParameters(param, 2) % virsh start linux error: Failed to start domain rhel6.3rc error: (domain_definition):8: error parsing attribute name memory mode='preferred'/numatune ---^ --- src/conf/domain_conf.c | 17 ++--- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81c6308..c44d89d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12795,23 +12795,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, const char *placement; mode = virDomainNumatuneMemModeTypeToString(def-numatune.memory.mode); -virBufferAsprintf(buf, memory mode='%s' , mode); +virBufferAsprintf(buf, memory mode='%s', mode); -if (def-numatune.memory.placement_mode == -VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { +if (def-numatune.memory.nodemask) { nodemask = virDomainCpuSetFormat(def-numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + VIR_DOMAIN_CPUMASK_LEN); if (nodemask == NULL) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to format nodeset for NUMA memory tuning)); goto cleanup; } -virBufferAsprintf(buf, nodeset='%s'/\n, nodemask); +virBufferAsprintf(buf, nodeset='%s'/\n, nodemask); VIR_FREE(nodemask); -} else if (def-numatune.memory.placement_mode) { +} else if (def-numatune.memory.placement_mode == +VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) { placement = virDomainNumatuneMemPlacementModeTypeToString(def-numatune.memory.placement_mode); -virBufferAsprintf(buf, placement='%s'/\n, placement); +virBufferAsprintf(buf, placement='%s'/\n, placement); +} else { +/* Should not hit here. */ +virBufferAddLit(buf, /\n); } virBufferAddLit(buf, /numatune\n); } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Default pool permission mode to 0711
And ping. On 2012年06月21日 11:49, Osier Yang wrote: On 2012年06月19日 00:24, Eric Blake wrote: On 06/18/2012 03:47 AM, Osier Yang wrote: Per the typical use of libvirt is to fork the qemu process with qemu:qemu. Setting the pool permission mode as 0700 by default will prevent the guest start with permission reason. Define macro for the default pool and vol permission modes incidentally. --- src/conf/storage_conf.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index bf4567f..6d4987b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -47,6 +47,8 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE +#define DEFAULT_POOL_PERM_MODE 0711 +#define DEFAULT_VOL_PERM_MODE 0600 Isn't 755 more typical than 711 for directory permissions? For that reason, I'd like a second opinion on whether the more relaxed permissions make sense. The difference is 755 allows the group users and others to inspect what the images are and their permissions in the pool. The side effect what I can think of is: % ls -l /var/lib/libvirt/images/ -rw-r--r--. 1 root root 1048576 6月 18 14:34 attch.img -rw-r--r--. 1 root root 1048576 6月 14 17:38 foo2.img -rw-r--r--. 1 root root 1048576 6月 14 17:33 foo.img -rw-rw-rw-. 1 root root 0 6月 21 11:31 local.img % /var/lib/libvirt/images/local.img I.e, if one can check the files in the pool, and the vols have write permission for group users/others exposed, then it can be easily damaged. However, one can destroy the vols data anyway even with 711, though one should known the filename of the target vol first, e.g. % ls -ld /var/lib/libvirt/images/ drwx--x--x. 2 root root 4096 Jun 18 14:34 /var/lib/libvirt/images/ % stat /var/lib/libvirt/images/local.img File: `/var/lib/libvirt/images/local.img' Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: 808h/2056d Inode: 1054167 Links: 1 Access: (0666/-rw-rw-rw-) Uid: ( 0/ root) Gid: ( 0/ root) Context: system_u:object_r:virt_image_t:s0 Access: 2012-06-21 11:39:41.928284645 +0800 Modify: 2012-06-21 11:31:11.948457979 +0800 Change: 2012-06-21 11:38:58.948639333 +0800 Birth: - % /var/lib/libvirt/images/local.img % So from my p.o.v, 711 is better choice, at least it's not that easy for the group users/others to get the file names in the pool. Regards, Osier -- 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] [PATCH 03/13] create a new cgroup and move all hypervisor threads to the new cgroup
hi~ On 07/03/2012 07:06 AM, Eric Blake wrote: On 06/05/2012 02:16 AM, tangchen wrote: create a new cgroup and move all hypervisor threads to the new cgroup. And then we can do the other things: 1. limit only vcpu usage rather than the whole qemu 2. limit for hypervisor threads(include vhost-net threads) A really useful thing to add to this commit message would be an ascii view of the cgroup hierarchy being created. If I understand correctly, this creates the following levels: cgroup mount point libvirt subdirectory (all libvirt management) driver subdirectory (all guests tied to one driver) hypervisor subdirectory (all processes tied to one guest) vcpu subdirectory (all processes tied to one VCPU of a guest) I would almost prefer to call it a VM cgroup instead of a hypervisor cgroup (and that reflects back to naming chosen in 2/13), as I tend to think of 'hypervisor' meaning 'qemu' - the technology that drives multiple guests, while I think of 'VM' meaning 'single guest', a collection of possible multiple processes under a single qemu process umbrella for running a given guest. Well, actually I see it as follow: cgroup mount point libvirt subdirectory (all libvirt management) driver subdirectory (all guests tied to one driver) VM subdirectory (all processes tied to one guest) vcpu subdirectory (all processes tied to one VCPU of a guest) hypervisor subdirectory So I think the name is fine. What do you think? Now, I didn't change anything here. But if you insist, I think we can discuss it farther. + +if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) { +/* If we does not know VCPU-PID mapping or all vcpu runs in the same s/vcpu runs/vcpus run/ + * thread, we cannot control each vcpu. + */ +virCgroupFree(cgroup); +return 0; It makes sense to ignore failure to set up a vcpu sub-cgroup if the user never requested the feature, with the end result being that they lose out on the functionality. But if the user explicitly requested per-vcpu usage and we can't set it up, should this return a failure? In other words, I'm worried whether we need to detect whether it is always safe to ignore the failure (as done here) or whether there are situations where setup failure should prevent running the VM until the cgroup situation is resolved. I report an error here, please refer to my new patches. Thanks. :) -- Best Regards, Tang chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] openvz: Handle domain obj hash map errors
On Tue, Jul 10, 2012 at 02:42:13PM +0800, Osier Yang wrote: On 2012年07月10日 13:21, Guido Günther wrote: This makes the driver fail with a clear error message in case of uuid collisions (for example if somebody copied a container configuration without updating the UUID). OpenVZ itself doesn't complain about duplicate UUIDs since this parameter is only used by libvirt. --- src/openvz/openvz_conf.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 503c8a0..0764c2c 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -684,8 +684,11 @@ int openvzLoadDomains(struct openvz_driver *driver) { openvzReadMemConf(dom-def, veid); virUUIDFormat(dom-def-uuid, uuidstr); -if (virHashAddEntry(driver-domains.objs, uuidstr, dom) 0) +if (virHashAddEntry(driver-domains.objs, uuidstr, dom) 0) { +openvzError(VIR_ERR_INTERNAL_ERROR, +_(Could not add UUID for container %d), veid); goto cleanup; +} It's good to have a clear error for the UUID collisions, but it's not the only reason for failure of virHashAddEntry. It could be the memory allocation error too. So The better fix is to check the UUID duplication explicitly with virHashLookup before adding it to the table, (virHashAddOrUpdateEntry reports the memory allocation error anyway, so we just need to care about the UUID conllisions, and actually the memory error could be overriden with this patch). Agreed, you need to check for collisions before adding the entry, otherwise you obscure other relevant error messages. 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 1/3] conf: Format numatune XML correctly while placement is none
On Tue, Jul 10, 2012 at 05:31:17PM +0800, Osier Yang wrote: ping? On 2012年06月25日 12:28, Osier Yang wrote: setNumaParameters tunes the numa setting using cgroup, it's another entry except libnuma/numad for numa tuning. And it doesn't set the placement, and further more, the formating codes doesn't take this into consideration. How to reproduce: conn = libvirt.open(None) dom = conn.lookupByName('linux') param = {'numa_nodeset': '0', 'numa_mode': 1} dom.setNumaParameters(param, 2) % virsh start linux error: Failed to start domain rhel6.3rc error: (domain_definition):8: error parsing attribute name memory mode='preferred'/numatune ---^ --- src/conf/domain_conf.c | 17 ++--- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81c6308..c44d89d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12795,23 +12795,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, const char *placement; mode = virDomainNumatuneMemModeTypeToString(def-numatune.memory.mode); -virBufferAsprintf(buf, memory mode='%s' , mode); +virBufferAsprintf(buf, memory mode='%s', mode); -if (def-numatune.memory.placement_mode == -VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { +if (def-numatune.memory.nodemask) { nodemask = virDomainCpuSetFormat(def-numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + VIR_DOMAIN_CPUMASK_LEN); if (nodemask == NULL) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to format nodeset for NUMA memory tuning)); goto cleanup; } -virBufferAsprintf(buf, nodeset='%s'/\n, nodemask); +virBufferAsprintf(buf, nodeset='%s'/\n, nodemask); VIR_FREE(nodemask); -} else if (def-numatune.memory.placement_mode) { +} else if (def-numatune.memory.placement_mode == +VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) { placement = virDomainNumatuneMemPlacementModeTypeToString(def-numatune.memory.placement_mode); -virBufferAsprintf(buf, placement='%s'/\n, placement); +virBufferAsprintf(buf, placement='%s'/\n, placement); +} else { +/* Should not hit here. */ +virBufferAddLit(buf, /\n); } virBufferAddLit(buf, /numatune\n); } The fact that this bug existed shows that the test suite for the XML parser is incomplete. Please resubmit with an extra test XML datafile for the test suite to validate this scenario. 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] Problem setting CPU topology
On Sat, Jul 07, 2012 at 07:10:53PM +0300, Zeeshan Ali (Khattak) wrote: Hi, I'm trying to set exact CPU topology to qemu-kvm domains to match host's topology. In my case, host topology is: 1 socket, 2 cores and 2 threads. If I set the XML like this: domain type='kvm' .. vcpu placement='static'4/vcpu os type arch='i686' machine='pc-0.15'hvm/type boot dev='hd'/ /os cpu mode='host-model' model fallback='allow'/ topology sockets='1' cores='2' threads='2'/ /cpu .. The qemu commandline launched for this domain looks like this: /usr/bin/qemu-kvm -name fedora17-2 -S -M pc-0.15 -cpu core2duo,+lahf_lm,+rdtscp,+aes,+popcnt,+sse4.2,+sse4.1,+pdcm,+xtpr,+cx16,+tm2,+est,+smx,+vmx,+ds_cpl,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+ds -enable-kvm -m 1152 -smp 4,sockets=1,cores=2,threads=2 -uuid c573342b-2876-05b8-098e-6d5314cab062 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/home/zeenix/.config/libvirt/qemu/lib/fedora17-2.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -no-kvm-pit-reinjection -no-shutdown -no-acpi I debugged this together with Zeeshan, and the issue comes from this -no-acpi flag which libvirt added because the domain XML was missing featuresacpi//features So in the end that was a bug in Boxes, not really a libvirt/qemu issue. Unless libvirt/qemu should be taught that CPU topology won't work without ACPI and complain about it? Christophe pgpeUvIGOPm9j.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Default pool permission mode to 0711
On 06/21/12 05:49, Osier Yang wrote: On 2012年06月19日 00:24, Eric Blake wrote: On 06/18/2012 03:47 AM, Osier Yang wrote: Per the typical use of libvirt is to fork the qemu process with qemu:qemu. Setting the pool permission mode as 0700 by default will prevent the guest start with permission reason. Define macro for the default pool and vol permission modes incidentally. --- src/conf/storage_conf.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index bf4567f..6d4987b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -47,6 +47,8 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE +#define DEFAULT_POOL_PERM_MODE 0711 +#define DEFAULT_VOL_PERM_MODE 0600 Isn't 755 more typical than 711 for directory permissions? For that reason, I'd like a second opinion on whether the more relaxed permissions make sense. The difference is 755 allows the group users and others to inspect what the images are and their permissions in the pool. The side effect what I can think of is: % ls -l /var/lib/libvirt/images/ -rw-r--r--. 1 root root 1048576 6月 18 14:34 attch.img -rw-r--r--. 1 root root 1048576 6月 14 17:38 foo2.img -rw-r--r--. 1 root root 1048576 6月 14 17:33 foo.img -rw-rw-rw-. 1 root root 0 6月 21 11:31 local.img % /var/lib/libvirt/images/local.img I.e, if one can check the files in the pool, and the vols have write permission for group users/others exposed, then it can be easily damaged. However, one can destroy the vols data anyway even with 711, though one should known the filename of the target vol first, e.g. By not allowing to view the directory contents you don't really add much security. I don't like security-by-obscurity approaches. IIUC you are able to change the permissions on the pool if you wish to have different from the default, so this choice should just % ls -ld /var/lib/libvirt/images/ drwx--x--x. 2 root root 4096 Jun 18 14:34 /var/lib/libvirt/images/ % stat /var/lib/libvirt/images/local.img File: `/var/lib/libvirt/images/local.img' Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: 808h/2056dInode: 1054167 Links: 1 Access: (0666/-rw-rw-rw-) Uid: (0/root) Gid: (0/root) Context: system_u:object_r:virt_image_t:s0 Access: 2012-06-21 11:39:41.928284645 +0800 Modify: 2012-06-21 11:31:11.948457979 +0800 Change: 2012-06-21 11:38:58.948639333 +0800 Birth: - % /var/lib/libvirt/images/local.img % So from my p.o.v, 711 is better choice, at least it's not that easy for the group users/others to get the file names in the pool. I vote for the more common 755 permissions. We shouldn't try to hide the real problem if permissions are misconfigured by hiding the names. Peter Regards, Osier -- 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] [PATCH v2 1/3] conf: Format numatune XML correctly while placement is none
On 2012年07月10日 17:41, Daniel P. Berrange wrote: On Tue, Jul 10, 2012 at 05:31:17PM +0800, Osier Yang wrote: ping? On 2012年06月25日 12:28, Osier Yang wrote: setNumaParameters tunes the numa setting using cgroup, it's another entry except libnuma/numad for numa tuning. And it doesn't set the placement, and further more, the formating codes doesn't take this into consideration. How to reproduce: conn = libvirt.open(None) dom = conn.lookupByName('linux') param = {'numa_nodeset': '0', 'numa_mode': 1} dom.setNumaParameters(param, 2) % virsh start linux error: Failed to start domain rhel6.3rc error: (domain_definition):8: error parsing attribute name memory mode='preferred'/numatune ---^ --- src/conf/domain_conf.c | 17 ++--- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81c6308..c44d89d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12795,23 +12795,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, const char *placement; mode = virDomainNumatuneMemModeTypeToString(def-numatune.memory.mode); -virBufferAsprintf(buf, memory mode='%s' , mode); +virBufferAsprintf(buf, memory mode='%s', mode); -if (def-numatune.memory.placement_mode == -VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { +if (def-numatune.memory.nodemask) { nodemask = virDomainCpuSetFormat(def-numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + VIR_DOMAIN_CPUMASK_LEN); if (nodemask == NULL) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to format nodeset for NUMA memory tuning)); goto cleanup; } -virBufferAsprintf(buf, nodeset='%s'/\n, nodemask); +virBufferAsprintf(buf, nodeset='%s'/\n, nodemask); VIR_FREE(nodemask); -} else if (def-numatune.memory.placement_mode) { +} else if (def-numatune.memory.placement_mode == +VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) { placement = virDomainNumatuneMemPlacementModeTypeToString(def-numatune.memory.placement_mode); -virBufferAsprintf(buf, placement='%s'/\n, placement); +virBufferAsprintf(buf, placement='%s'/\n, placement); +} else { +/* Should not hit here. */ +virBufferAddLit(buf, /\n); } virBufferAddLit(buf, /numatune\n); } The fact that this bug existed shows that the test suite for the XML parser is incomplete. Please resubmit with an extra test XML datafile for the test suite to validate this scenario. we already has good enough XMLs for the test suite: % ls tests/qemuxml2argvdata/qemuxml2argv-numa qemuxml2argv-numad.args qemuxml2argv-numad-auto-vcpu-static-numatune.xml qemuxml2argv-numad-auto-memory-vcpu-cpuset.args qemuxml2argv-numad-static-memory-auto-vcpu.args qemuxml2argv-numad-auto-memory-vcpu-cpuset.xml qemuxml2argv-numad-static-memory-auto-vcpu.xml qemuxml2argv-numad-auto-memory-vcpu-no-cpuset-and-placement.args qemuxml2argv-numad-static-vcpu-no-numatune.xml qemuxml2argv-numad-auto-memory-vcpu-no-cpuset-and-placement.xml qemuxml2argv-numad.xml qemuxml2argv-numad-auto-vcpu-no-numatune.xml qemuxml2argv-numatune-memory.args qemuxml2argv-numad-auto-vcpu-static-numatune.args qemuxml2argv-numatune-memory.xml The problem is we have two entries to change the numatune config, and they share the same XML syntax (btw, I was thinking it's a bad idea to do so, it could just cause crasy results). XML parser actually ensures the placement mode can be always set with either 'static' or 'auto', but API via cgroup don't set placement mode as placement is meaningless for it, so IMHO no need to add XMLs to test the parser, instead we need to add tests to test the API. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix /domain/features setting in qemuParseCommandLine
Commit 5e6ce1 moved down detection of the ACPI feature in qemuParseCommandLine. However, when ACPI is detected, it clears all feature flags in def-features to only set ACPI. This used to be fine because this was the first place were def-features was set, but after the move this is no longer necessarily true because this block comes before the ACPI check: if (strstr(def-emulator, kvm)) { def-virtType = VIR_DOMAIN_VIRT_KVM; def-features |= (1 VIR_DOMAIN_FEATURE_PAE); } Since def is allocated in qemuParseCommandLine using VIR_ALLOC, we can always use |= when modifying def-features --- This is an issue I noticed while reading this code for other reasons, I have only compile-tested it and I'm not sure how to test it. Christophe src/qemu/qemu_command.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6549f57..0065e83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7547,7 +7547,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; if (STREQ(def-os.arch, i686)||STREQ(def-os.arch, x86_64)) -def-features = (1 VIR_DOMAIN_FEATURE_ACPI) +def-features |= (1 VIR_DOMAIN_FEATURE_ACPI) /*| (1 VIR_DOMAIN_FEATURE_APIC)*/; #define WANT_VALUE() \ const char *val = progargv[++i]; \ -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/4] nodeinfo_test: Enhance test data before changing nodeinfo gathering
On 09.07.2012 17:17, Peter Krempa wrote: This patch adds test data needed by the new way node information will be gathered. This patch adds symlinks to cpu cores to their corresponding node directory. --- .../linux-nodeinfo-sysfs-test-2/node/node0/cpu0|1 + .../linux-nodeinfo-sysfs-test-2/node/node0/cpu1|1 + .../linux-nodeinfo-sysfs-test-3/node/node0/cpu0|1 + .../linux-nodeinfo-sysfs-test-3/node/node0/cpu12 |1 + .../linux-nodeinfo-sysfs-test-3/node/node0/cpu16 |1 + .../linux-nodeinfo-sysfs-test-3/node/node0/cpu20 |1 + .../linux-nodeinfo-sysfs-test-3/node/node0/cpu4|1 + .../linux-nodeinfo-sysfs-test-3/node/node0/cpu8|1 + .../linux-nodeinfo-sysfs-test-3/node/node1/cpu24 |1 + .../linux-nodeinfo-sysfs-test-3/node/node1/cpu28 |1 + .../linux-nodeinfo-sysfs-test-3/node/node1/cpu32 |1 + .../linux-nodeinfo-sysfs-test-3/node/node1/cpu36 |1 + .../linux-nodeinfo-sysfs-test-3/node/node1/cpu40 |1 + .../linux-nodeinfo-sysfs-test-3/node/node1/cpu44 |1 + .../linux-nodeinfo-sysfs-test-3/node/node2/cpu11 |1 + .../linux-nodeinfo-sysfs-test-3/node/node2/cpu15 |1 + .../linux-nodeinfo-sysfs-test-3/node/node2/cpu19 |1 + .../linux-nodeinfo-sysfs-test-3/node/node2/cpu23 |1 + .../linux-nodeinfo-sysfs-test-3/node/node2/cpu3|1 + .../linux-nodeinfo-sysfs-test-3/node/node2/cpu7|1 + .../linux-nodeinfo-sysfs-test-3/node/node3/cpu27 |1 + .../linux-nodeinfo-sysfs-test-3/node/node3/cpu31 |1 + .../linux-nodeinfo-sysfs-test-3/node/node3/cpu35 |1 + .../linux-nodeinfo-sysfs-test-3/node/node3/cpu39 |1 + .../linux-nodeinfo-sysfs-test-3/node/node3/cpu43 |1 + .../linux-nodeinfo-sysfs-test-3/node/node3/cpu47 |1 + .../linux-nodeinfo-sysfs-test-3/node/node4/cpu10 |1 + .../linux-nodeinfo-sysfs-test-3/node/node4/cpu14 |1 + .../linux-nodeinfo-sysfs-test-3/node/node4/cpu18 |1 + .../linux-nodeinfo-sysfs-test-3/node/node4/cpu2|1 + .../linux-nodeinfo-sysfs-test-3/node/node4/cpu22 |1 + .../linux-nodeinfo-sysfs-test-3/node/node4/cpu6|1 + .../linux-nodeinfo-sysfs-test-3/node/node5/cpu26 |1 + .../linux-nodeinfo-sysfs-test-3/node/node5/cpu30 |1 + .../linux-nodeinfo-sysfs-test-3/node/node5/cpu34 |1 + .../linux-nodeinfo-sysfs-test-3/node/node5/cpu38 |1 + .../linux-nodeinfo-sysfs-test-3/node/node5/cpu42 |1 + .../linux-nodeinfo-sysfs-test-3/node/node5/cpu46 |1 + .../linux-nodeinfo-sysfs-test-3/node/node6/cpu1|1 + .../linux-nodeinfo-sysfs-test-3/node/node6/cpu13 |1 + .../linux-nodeinfo-sysfs-test-3/node/node6/cpu17 |1 + .../linux-nodeinfo-sysfs-test-3/node/node6/cpu21 |1 + .../linux-nodeinfo-sysfs-test-3/node/node6/cpu5|1 + .../linux-nodeinfo-sysfs-test-3/node/node6/cpu9|1 + .../linux-nodeinfo-sysfs-test-3/node/node7/cpu25 |1 + .../linux-nodeinfo-sysfs-test-3/node/node7/cpu29 |1 + .../linux-nodeinfo-sysfs-test-3/node/node7/cpu33 |1 + .../linux-nodeinfo-sysfs-test-3/node/node7/cpu37 |1 + .../linux-nodeinfo-sysfs-test-3/node/node7/cpu41 |1 + .../linux-nodeinfo-sysfs-test-3/node/node7/cpu45 |1 + .../linux-nodeinfo-sysfs-test-3/node/possible | Bin 4 - 5 bytes 51 files changed, 50 insertions(+), 0 deletions(-) create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-2/node/node0/cpu0 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-2/node/node0/cpu1 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node0/cpu0 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node0/cpu12 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node0/cpu16 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node0/cpu20 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node0/cpu4 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node0/cpu8 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node1/cpu24 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node1/cpu28 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node1/cpu32 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node1/cpu36 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node1/cpu40 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node1/cpu44 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node2/cpu11 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node2/cpu15 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node2/cpu19 create mode 12 tests/nodeinfodata/linux-nodeinfo-sysfs-test-3/node/node2/cpu23
Re: [libvirt] [PATCHv3 4/4] test: Add test case for nodeinfotest if host machine doesn't have NUMA
On 09.07.2012 17:17, Peter Krempa wrote: Test filling of nodeinfo structure if /sys/devices/system/node does not exist. (Based on dump from a real machine) --- .../linux-nodeinfo-sysfs-test-5-cpu-x86-output.txt |1 + .../linux-nodeinfo-sysfs-test-5-x86.cpuinfo| 100 .../cpu/cpu0/topology/core_id |1 + .../cpu/cpu0/topology/core_siblings|1 + .../cpu/cpu0/topology/core_siblings_list |1 + .../cpu/cpu0/topology/physical_package_id |1 + .../cpu/cpu0/topology/thread_siblings |1 + .../cpu/cpu0/topology/thread_siblings_list |1 + .../linux-nodeinfo-sysfs-test-5/cpu/cpu1/online|1 + .../cpu/cpu1/topology/core_id |1 + .../cpu/cpu1/topology/core_siblings|1 + .../cpu/cpu1/topology/core_siblings_list |1 + .../cpu/cpu1/topology/physical_package_id |1 + .../cpu/cpu1/topology/thread_siblings |1 + .../cpu/cpu1/topology/thread_siblings_list |1 + .../linux-nodeinfo-sysfs-test-5/cpu/cpu2/online|1 + .../cpu/cpu2/topology/core_id |1 + .../cpu/cpu2/topology/core_siblings|1 + .../cpu/cpu2/topology/core_siblings_list |1 + .../cpu/cpu2/topology/physical_package_id |1 + .../cpu/cpu2/topology/thread_siblings |1 + .../cpu/cpu2/topology/thread_siblings_list |1 + .../linux-nodeinfo-sysfs-test-5/cpu/cpu3/online|1 + .../cpu/cpu3/topology/core_id |1 + .../cpu/cpu3/topology/core_siblings|1 + .../cpu/cpu3/topology/core_siblings_list |1 + .../cpu/cpu3/topology/physical_package_id |1 + .../cpu/cpu3/topology/thread_siblings |1 + .../cpu/cpu3/topology/thread_siblings_list |1 + tests/nodeinfotest.c |1 + 30 files changed, 129 insertions(+), 0 deletions(-) create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5-cpu-x86-output.txt create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5-x86.cpuinfo create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu0/topology/core_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu0/topology/core_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu0/topology/core_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu0/topology/physical_package_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu0/topology/thread_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu0/topology/thread_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/online create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/topology/core_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/topology/core_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/topology/core_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/topology/physical_package_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/topology/thread_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu1/topology/thread_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/online create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/topology/core_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/topology/core_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/topology/core_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/topology/physical_package_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/topology/thread_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu2/topology/thread_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu3/online create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu3/topology/core_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu3/topology/core_siblings create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu3/topology/core_siblings_list create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu3/topology/physical_package_id create mode 100644 tests/nodeinfodata/linux-nodeinfo-sysfs-test-5/cpu/cpu3/topology/thread_siblings create mode 100644
Re: [libvirt] [PATCHv3 2/4] nodeinfo: Fix gathering of nodeinfo data structure
On 09.07.2012 17:17, Peter Krempa wrote: This patch changes the way data to fill the nodeinfo structure are gathered. We've gathere the test data by iterating processors an sockets separately from nodes. The reported data was based solely on information about core id. Problems arise when eg cores in mulit-processor machines don't have same id's on both processors or maybe one physical processor contains more NUMA nodes. This patch changes the approach how we detect processors and nodes. Now we start at enumerating nodes and for each node processors, sockets and threads are enumerated separately. This approach provides acurate data that comply to docs about the nodeinfo structure. This also enables to get rid of hacks: see commits 10d9038b744a69c8d4bd29c2e8c012a097481586, ac9dd4a676f21b5e3ca6dbe0526f2a6709072beb. (Those changes in nodeinfo.c are efectively reverted by this patch). This patch also changes output of one of the tests, as the processor topology is now acquired more precisely. --- src/nodeinfo.c | 311 .../linux-nodeinfo-sysfs-test-3-cpu-x86-output.txt |2 +- 2 files changed, 185 insertions(+), 128 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 819f954..ae713da 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -188,38 +188,131 @@ virNodeParseSocket(const char *dir, unsigned int cpu) return ret; } +/* parses a node entry, returning number of processors in the node and + * filling arguments */ static int -virNodeParseNode(const char *sysfs_dir) +virNodeParseNode(const char *node, int *sockets, int *cores, int *threads) I'd mark these as ATTRIBUTE_NONNULL esp when ... { -char *file = NULL; -char *possible = NULL; -char *tmp; int ret = -1; +int processors = 0; +DIR *cpudir = NULL; +struct dirent *cpudirent = NULL; +int sock_max = 0; +cpu_set_t sock_map; +int sock; +cpu_set_t *core_maps = NULL; +int core; +int i; +int siblings; +unsigned int cpu; +int online; -if (virAsprintf(file, %s/node/possible, sysfs_dir) 0) { -virReportOOMError(); +*threads = 0; +*cores = 0; +*sockets = 0; ... doing this. + +if (!(cpudir = opendir(node))) { +virReportSystemError(errno, _(cannot opendir %s), node); goto cleanup; } -/* Assume that a missing node/possible file implies no NUMA - * support, and hence all cpus belong to the same node. */ -if (!virFileExists(file)) { -ret = 1; + +/* enumerate sockets in the node */ +CPU_ZERO(sock_map); +while ((cpudirent = readdir(cpudir))) { I guess we want reentrant version of readdir here, don't we? +if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1) +continue; + +/* Parse socket */ +sock = virNodeParseSocket(node, cpu); +CPU_SET(sock, sock_map); + +if (sock sock_max) +sock_max = sock; +} + +if (errno) { You should have reset errno before while() loop. +virReportSystemError(errno, _(problem reading %s), node); goto cleanup; } -if (virFileReadAll(file, 1024, possible) 0) + +sock_max++; + +/* allocate cpu maps for each socket */ +if (VIR_ALLOC_N(core_maps, sock_max) 0) { +virReportOOMError(); goto cleanup; -if (virStrToLong_i(possible, tmp, 10, ret) 0 || -(*tmp == '-' virStrToLong_i(tmp+1, tmp, 10, ret) 0) || -*tmp != '\n') { -nodeReportError(VIR_ERR_INTERNAL_ERROR, -_(failed to parse possible nodes '%s'), possible); +} + +for (i = 0; i sock_max; i++) +CPU_ZERO(core_maps[i]); + +/* iterate over all CPU's in the node */ +rewinddir(cpudir); +while ((cpudirent = readdir(cpudir))) { Again, s/readdir/readdir_r/ +if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1) +continue; + +if ((online = virNodeGetCpuValue(node, cpu, online, true)) 0) +goto cleanup; + +if (!online) +continue; + +processors++; + +/* Parse socket */ +sock = virNodeParseSocket(node, cpu); +if (!CPU_ISSET(sock, sock_map)) { +nodeReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(CPU socket topology has changed)); +goto cleanup; +} + +/* Parse core */ +# if defined(__s390__) || \ +defined(__s390x__) +/* logical cpu is equivalent to a core on s390 */ Bad indentation. +core = cpu; +# else +core = virNodeGetCpuValue(node, cpu, topology/core_id, false); +# endif + +CPU_SET(core, core_maps[sock]); + +if (!(siblings = virNodeCountThreadSiblings(node, cpu))) +goto cleanup; + +if (siblings *threads) +*threads =
Re: [libvirt] [PATCHv3 3/4] test: Add new test case for nodeinfotest
On 09.07.2012 17:17, Peter Krempa wrote: This patch adds test data that describe a machine that has two physical processors that don't share same core id's on their cores. On this data the virsh nodeinfo reported that the machine had 10 cores per socket while the processor had only 8. (Before fixing nodeinfo gathering code). --- .../linux-nodeinfo-sysfs-test-4-cpu-x86-output.txt |1 + .../linux-nodeinfo-sysfs-test-4-x86.cpuinfo| 400 .../cpu/cpu0/topology/core_id |1 + .../cpu/cpu0/topology/physical_package_id |1 + .../cpu/cpu0/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu1/online|1 + .../cpu/cpu1/topology/core_id |1 + .../cpu/cpu1/topology/physical_package_id |1 + .../cpu/cpu1/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu10/online |1 + .../cpu/cpu10/topology/core_id |1 + .../cpu/cpu10/topology/physical_package_id |1 + .../cpu/cpu10/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu11/online |1 + .../cpu/cpu11/topology/core_id |1 + .../cpu/cpu11/topology/physical_package_id |1 + .../cpu/cpu11/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu12/online |1 + .../cpu/cpu12/topology/core_id |1 + .../cpu/cpu12/topology/physical_package_id |1 + .../cpu/cpu12/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu13/online |1 + .../cpu/cpu13/topology/core_id |1 + .../cpu/cpu13/topology/physical_package_id |1 + .../cpu/cpu13/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu14/online |1 + .../cpu/cpu14/topology/core_id |1 + .../cpu/cpu14/topology/physical_package_id |1 + .../cpu/cpu14/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu15/online |1 + .../cpu/cpu15/topology/core_id |1 + .../cpu/cpu15/topology/physical_package_id |1 + .../cpu/cpu15/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu2/online|1 + .../cpu/cpu2/topology/core_id |1 + .../cpu/cpu2/topology/physical_package_id |1 + .../cpu/cpu2/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu3/online|1 + .../cpu/cpu3/topology/core_id |1 + .../cpu/cpu3/topology/physical_package_id |1 + .../cpu/cpu3/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu4/online|1 + .../cpu/cpu4/topology/core_id |1 + .../cpu/cpu4/topology/physical_package_id |1 + .../cpu/cpu4/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu5/online|1 + .../cpu/cpu5/topology/core_id |1 + .../cpu/cpu5/topology/physical_package_id |1 + .../cpu/cpu5/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu6/online|1 + .../cpu/cpu6/topology/core_id |1 + .../cpu/cpu6/topology/physical_package_id |1 + .../cpu/cpu6/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu7/online|1 + .../cpu/cpu7/topology/core_id |1 + .../cpu/cpu7/topology/physical_package_id |1 + .../cpu/cpu7/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu8/online|1 + .../cpu/cpu8/topology/core_id |1 + .../cpu/cpu8/topology/physical_package_id |1 + .../cpu/cpu8/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/cpu/cpu9/online|1 + .../cpu/cpu9/topology/core_id |1 + .../cpu/cpu9/topology/physical_package_id |1 + .../cpu/cpu9/topology/thread_siblings |1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu0|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu1|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu2|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu3|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu4|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu5|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu6|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/cpu7|1 + .../linux-nodeinfo-sysfs-test-4/node/node0/meminfo | 29 ++ .../linux-nodeinfo-sysfs-test-4/node/node1/cpu10
[libvirt] [PATCH] virsh: Ensure the parents of the readline history path exists
Instead of changing the existed virFileMakePath to accept mode argument and modifying a pile of its uses, this patch introduces virFileMakePathWithMode, and use it instead of mkdir() to create the readline history dir. --- src/libvirt_private.syms |1 + src/util/util.c | 15 +++ src/util/util.h |2 ++ tools/virsh.c|3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6625fc6..b173590 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1146,6 +1146,7 @@ virFileIsDir; virFileLinkPointsTo; virFileLock; virFileMakePath; +virFileMakePathWithMode; virFileMatchesNameSuffix; virFileOpenAs; virFileOpenTty; diff --git a/src/util/util.c b/src/util/util.c index f886ea7..47b1366 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1248,7 +1248,7 @@ int virDirCreate(const char *path ATTRIBUTE_UNUSED, } #endif /* WIN32 */ -static int virFileMakePathHelper(char *path) +static int virFileMakePathHelper(char *path, mode_t mode) { struct stat st; char *p; @@ -1272,13 +1272,13 @@ static int virFileMakePathHelper(char *path) if (p != path) { *p = '\0'; -if (virFileMakePathHelper(path) 0) +if (virFileMakePathHelper(path, mode) 0) return -1; *p = '/'; } -if (mkdir(path, 0777) 0 errno != EEXIST) +if (mkdir(path, mode) 0 errno != EEXIST) return -1; return 0; @@ -1292,13 +1292,20 @@ static int virFileMakePathHelper(char *path) */ int virFileMakePath(const char *path) { +return virFileMakePathWithMode(path, 0777); +} + +int +virFileMakePathWithMode(const char *path, +mode_t mode) +{ int ret = -1; char *tmp; if ((tmp = strdup(path)) == NULL) goto cleanup; -ret = virFileMakePathHelper(tmp); +ret = virFileMakePathHelper(tmp, mode); cleanup: VIR_FREE(tmp); diff --git a/src/util/util.h b/src/util/util.h index 0af7e6d..33226b2 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -115,6 +115,8 @@ enum { int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK; +int virFileMakePathWithMode(const char *path, +mode_t mode) ATTRIBUTE_RETURN_CHECK; char *virFileBuildPath(const char *dir, const char *name, diff --git a/tools/virsh.c b/tools/virsh.c index 9008837..01e7ce0 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -20627,7 +20627,8 @@ static void vshReadlineDeinit (vshControl *ctl) { if (ctl-historyfile != NULL) { -if (mkdir(ctl-historydir, 0755) 0 errno != EEXIST) { +if (virFileMakePathWithMode(ctl-historydir, 0755) 0 +errno != EEXIST) { char ebuf[1024]; vshError(ctl, _(Failed to create '%s': %s), ctl-historydir, virStrerror(errno, ebuf, sizeof(ebuf))); -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Added timestamps to volumes
The access, modify and change times are added to volumes and corresponding xml representations. --- docs/schemas/storagevol.rng | 17 + src/conf/storage_conf.c |9 + src/conf/storage_conf.h |9 + src/storage/storage_backend.c |4 4 files changed, 39 insertions(+) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 7a74331..fc7eb09 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -63,6 +63,22 @@ /optional /define + define name='timestamps' +optional + element name='timestamps' +element name='atime' + ref name='unsignedLong'/ +/element +element name='mtime' + ref name='unsignedLong'/ +/element +element name='ctime' + ref name='unsignedLong'/ +/element + /element +/optional + /define + define name='target' element name='target' optional @@ -72,6 +88,7 @@ /optional ref name='format'/ ref name='permissions'/ + ref name='timestamps'/ optional ref name='encryption'/ /optional diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ab8df9e..a4cdac8 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1272,6 +1272,15 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAddLit(buf,/permissions\n); +virBufferAddLit(buf,timestamps\n); +virBufferAsprintf(buf, atime%llu/atime\n, + (unsigned long long) def-timestamps.atime); +virBufferAsprintf(buf, mtime%llu/mtime\n, + (unsigned long long) def-timestamps.mtime); +virBufferAsprintf(buf, ctime%llu/ctime\n, + (unsigned long long) def-timestamps.ctime); +virBufferAddLit(buf,/timestamps\n); + if (def-encryption) { virBufferAdjustIndent(buf, 4); if (virStorageEncryptionFormat(buf, def-encryption) 0) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 5733b57..8cd1d63 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -46,6 +46,14 @@ struct _virStoragePerms { /* Storage volumes */ +typedef struct _virStorageTimestamps virStorageTimestamps; +typedef virStorageTimestamps *virStorageTimestampsPtr; +struct _virStorageTimestamps { +time_t atime; +time_t mtime; +time_t ctime; +}; + /* * How the volume's data is stored on underlying @@ -77,6 +85,7 @@ struct _virStorageVolTarget { char *path; int format; virStoragePerms perms; +virStorageTimestamps timestamps; int type; /* only used by disk backend for partition type */ /* Currently used only in virStorageVolDef.target, not in .backingstore. */ virStorageEncryptionPtr encryption; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2e9b51..c827e3c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1209,6 +1209,10 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, target-perms.uid = sb.st_uid; target-perms.gid = sb.st_gid; +target-timestamps.atime = sb.st_atime; +target-timestamps.mtime = sb.st_mtime; +target-timestamps.ctime = sb.st_ctime; + VIR_FREE(target-perms.label); #if HAVE_SELINUX -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Use current uid and gid if they are passed as -1 for virDirCreate
On 21.06.2012 09:11, Osier Yang wrote: All the callers of virDirCreate are updated incidentally. --- src/storage/storage_backend_fs.c | 23 ++- src/util/util.c |6 ++ 2 files changed, 12 insertions(+), 17 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] domain_conf: Add USB controler model none
On 07/10/2012 01:01 AM, Osier Yang wrote: On 2012年07月10日 01:29, Peter Krempa wrote: Libvirt adds a USB controller to the guest even if the user does not specify any in the XML. This is due to back-compat reasons. It's not for back-compat reasons as far as I known, 42043afcd adds the implicit USB controller for virt-manager's use, and it causes the regression bug of migration, as the old libvirt doesn't have a default USB controller as a force. Jirka fixed it by commit 409b5f549. It IS for back-compat reasons. We have been outputting '-usb' in the qemu command line for ages prior to 42043afcd, and need a way to explicitly omit the '-usb' in the command line. Older libvirt had no way to represent the implicit device, and the fix of 409b5f549 was more of a hack that said when migrating back to an older libvirt, the existing usb controller of the sending libvirt would be implicitly recreated by the receiving libvirt. If a user explicitly requests the 'none' controller, then we must ensure that migration to an older host fails up front; the 'none' element must be explicit because the absence of an element implies the default usb controller. Is it a hard requirement of specify the 'none' USB controller model? Yes, just like it is a hard requirement that a user explicitly specifies the 'none' memoryballoon controller. That's the back-compat issue we have to face any time we make the XML explicit for something that older libvirt provided implicitly. This patch looks correct to me. -- 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] docs: added description of the vendor_id attribute
On 07/10/2012 03:26 AM, Michal Privoznik wrote: On 10.07.2012 10:01, Hendrik Schwartke wrote: --- docs/formatdomain.html.in |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94c555f..b6e0d5d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -779,7 +779,11 @@ in which case an attempt to start a domain requesting an unsupported CPU model will fail. Supported values for codefallback/code attribute are: codeallow/code (this is the default), and -codeforbid/code./dd +codeforbid/code. The optional codevendor_id/code attribute +(span class=sinceSince 0.9.14/span) can be used to set the I think our goal is to number the next release 0.10.0 (thanks to the addition of a new driver); but we can do a global search-and-replace of any 0.9.14 stragglers at once. -- 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 01/13] Introduce the function virCgroupForHypervisor
The subject should include 'v2' to mention that this is a respin; you can do that with 'git send-email --subject-prefix=PATCHv2'. On 07/10/2012 03:11 AM, tangchen wrote: Introduce the function virCgroupForHypervisor() to create sub directory for hypervisor thread(include I/O thread, vhost-net thread) So if I understand correctly, the intent is to create this hierarchy: cgroup mount point(s) (top-level controllers) - libvirt (subdirectory for all libvirt cgroups) --- qemu (all groups tied to a driver, virCgroupForDriver) - $name (all groups tied to a VM, virCgroupForDomain) --- $vcpuN (one group per vcpu, virCgroupForVcpu) --- hypervisor (one catchall group for non-vcpu, virCgroupForHypervisor) where hypervisor and vcpuN directories are siblings, both adding up to the sum total of $name for the VM totals. If so, please document that in the commit message. Signed-off-by: Wen Congyang we...@cn.fujitsu.com Still doesn't have the correct git authorship; git send-email would automatically insert a line: From: Wen Congyang we...@cn.fujitsu.com if you had correctly attributed authorship. I can fix that as needed, though. --- .gnulib |2 +- Oops; gnulib submodule updates should generally be an independent patch, it looks like it was an accident that you bumped it on this patch. Other than that, this patch looks okay. -- 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] [PATCHv3 3/4] test: Add new test case for nodeinfotest
On 07/10/2012 04:56 AM, Michal Privoznik wrote: On 09.07.2012 17:17, Peter Krempa wrote: This patch adds test data that describe a machine that has two physical processors that don't share same core id's on their cores. On this data the virsh nodeinfo reported that the machine had 10 cores per socket while the processor had only 8. (Before fixing nodeinfo gathering code). +fpu : yes +fpu_exception : yes +cpuid level : 11 +wp : yes +flags : fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good xtopology nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm dca sse4_1 sse4_2 x2apic popcnt aes lahf_lm ida arat epb dts tpr_shadow vnmi flexpriority ept vpid +bogomips: 5319.83 +clflush size: 64 +cache_alignment : 64 +address sizes : 44 bits physical, 48 bits virtual +power management: + Don't add empty newline here. We have an explicit whitespace exemption for these sorts of files, so that they can match actual machine outputs with their trailing empty line. This particular file will not trigger either a syntax check error or a commit hook rejection. ACK modulo empty newline. Empty newline is fine in this case, actually. -- 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] [PATCHv3 2/4] nodeinfo: Fix gathering of nodeinfo data structure
On 07/10/2012 04:56 AM, Michal Privoznik wrote: + +/* enumerate sockets in the node */ +CPU_ZERO(sock_map); +while ((cpudirent = readdir(cpudir))) { I guess we want reentrant version of readdir here, don't we? readdir_r() is a GNU extension not provided by gnulib, so we can't use it. Furthermore, readdir() is thread-safe if you have only one thread traversing a given DIR*; readdir_r() is only useful if you want to have multiple threads visit the same DIR* (which is not what we are doing here). For that reason, our syntax check does not forbid readdir, and we don't need to use readdir_r. +if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1) +continue; + +/* Parse socket */ +sock = virNodeParseSocket(node, cpu); +CPU_SET(sock, sock_map); + +if (sock sock_max) +sock_max = sock; +} + +if (errno) { You should have reset errno before while() loop. That part is true - you MUST reset errno before every call to readdir(), as it is the only way to tell errors apart from end-of-iteration. +/* iterate over all CPU's in the node */ +rewinddir(cpudir); +while ((cpudirent = readdir(cpudir))) { Again, s/readdir/readdir_r/ Overkill, not necessary. -- 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] [PATCHv3 4/4] test: Add test case for nodeinfotest if host machine doesn't have NUMA
On 07/10/2012 04:56 AM, Michal Privoznik wrote: On 09.07.2012 17:17, Peter Krempa wrote: Test filling of nodeinfo structure if /sys/devices/system/node does not exist. (Based on dump from a real machine) --- +bogomips: 3723.85 +clflush size: 64 +cache_alignment : 64 +address sizes : 36 bits physical, 48 bits virtual +power management: + Again, ACK modulo this ^^ empty newline. Again, empty newline is fine, since this file already has an explicit exemption. -- 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] virsh: Ensure the parents of the readline history path exists
On 07/10/2012 05:33 AM, Osier Yang wrote: Instead of changing the existed virFileMakePath to accept mode argument and modifying a pile of its uses, this patch introduces virFileMakePathWithMode, and use it instead of mkdir() to create the readline history dir. --- src/libvirt_private.syms |1 + src/util/util.c | 15 +++ src/util/util.h |2 ++ tools/virsh.c|3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) Does this always do the right thing? Remember, 'mkdir -p a/b/c' has the ability to create parent directories 'a' and 'a/b' with a different mode than the final directory 'a/b/c'; it is often the case that you want the parent directories to have more permissions than the final child. +++ b/tools/virsh.c @@ -20627,7 +20627,8 @@ static void vshReadlineDeinit (vshControl *ctl) { if (ctl-historyfile != NULL) { -if (mkdir(ctl-historydir, 0755) 0 errno != EEXIST) { +if (virFileMakePathWithMode(ctl-historydir, 0755) 0 But if we are happy with all intermediate directories being created mode 0755 instead of the default of 0777 from the original virFileMakePath, then this patch makes sense. 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] Added timestamps to volumes
On 07/10/2012 05:52 AM, Hendrik Schwartke wrote: The access, modify and change times are added to volumes and corresponding xml representations. --- docs/schemas/storagevol.rng | 17 + Incomplete. You must also document this in docs/formatstorage.html.in before this patch can be applied. src/conf/storage_conf.c |9 + src/conf/storage_conf.h |9 + src/storage/storage_backend.c |4 4 files changed, 39 insertions(+) +++ b/docs/schemas/storagevol.rng @@ -63,6 +63,22 @@ /optional /define + define name='timestamps' +optional + element name='timestamps' +element name='atime' + ref name='unsignedLong'/ +/element +element name='mtime' + ref name='unsignedLong'/ +/element +element name='ctime' + ref name='unsignedLong'/ +/element + /element +/optional Sounds interesting. Should we also list birth-time, which is available from some filesystems as a fourth timestamp? On BSD and Cygwin, birthtime is available as part of stat(); on Linux, you still have to use ioctl() or wait for the proposed xstat() interface to ever be finalized, and even then, not all file systems track that information, so it would have to be an optional element. I also think we need to track things to full precision, as in seconds + nanoseconds since epoch. That is, a proper timestamp would look like 1341925773.351768083 on a file system with full 9-digit sub-second resolution. +virBufferAddLit(buf,timestamps\n); Space after comma, throughout the patch. +typedef virStorageTimestamps *virStorageTimestampsPtr; +struct _virStorageTimestamps { +time_t atime; Store this as struct timespec, to match the POSIX definition of st_atim having both seconds and nanoseconds as part of stat() (oh phooey - the gnulib module stat-time for portably getting at nanoseconds is currently LGPLv3+; I'll see about whether we can get that relaxed). -- 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] virsh: Ensure the parents of the readline history path exists
On 2012年07月10日 21:04, Eric Blake wrote: On 07/10/2012 05:33 AM, Osier Yang wrote: Instead of changing the existed virFileMakePath to accept mode argument and modifying a pile of its uses, this patch introduces virFileMakePathWithMode, and use it instead of mkdir() to create the readline history dir. --- src/libvirt_private.syms |1 + src/util/util.c | 15 +++ src/util/util.h |2 ++ tools/virsh.c|3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) Does this always do the right thing? Remember, 'mkdir -p a/b/c' has the ability to create parent directories 'a' and 'a/b' with a different mode than the final directory 'a/b/c'; it is often the case that you want the parent directories to have more permissions than the final child. Oh, right. But looks like we don't have the requirement to set different mode for each parents yet. I pushed the patch, thanks for the reviewing! Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [libvirt-java] Fix typo in Domain.java.
OY == Osier Yang jy...@redhat.com writes: OY ACK and pushed. AUTHORS is updated with Claudio Bley OY cb...@av-test.de, please let us known if you have prefered OY name. Thanks. The name you put in is fine. Best regards, Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:http://www.av-test.org Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Use current uid and gid if they are passed as -1 for virDirCreate
On 2012年07月10日 20:06, Michal Privoznik wrote: On 21.06.2012 09:11, Osier Yang wrote: All the callers of virDirCreate are updated incidentally. --- src/storage/storage_backend_fs.c | 23 ++- src/util/util.c |6 ++ 2 files changed, 12 insertions(+), 17 deletions(-) ACK Michal Thanks, Pushed. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] dnsmasq didn't work as except
On 10.07.2012 15:55, ta...@linux.vnet.ibm.com wrote: hi all The vm created by libvirtd can not acquire a ip from dnsmasq. nobody 14203 1 0 21:31 ?00:00:00 /usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --conf-file= --except-interface lo --listen-address 192.168.122.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override note that --conf-file= is empty ,is there any thing I missing? No. That's intentional. thanks Eli. Well, maybe somebody is throwing away your DHCP requests. This command line doesn't look suspicious to me. Michal. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix /domain/features setting in qemuParseCommandLine
On 10.07.2012 12:14, Christophe Fergeau wrote: Commit 5e6ce1 moved down detection of the ACPI feature in qemuParseCommandLine. However, when ACPI is detected, it clears all feature flags in def-features to only set ACPI. This used to be fine because this was the first place were def-features was set, but after the move this is no longer necessarily true because this block comes before the ACPI check: if (strstr(def-emulator, kvm)) { def-virtType = VIR_DOMAIN_VIRT_KVM; def-features |= (1 VIR_DOMAIN_FEATURE_PAE); } Since def is allocated in qemuParseCommandLine using VIR_ALLOC, we can always use |= when modifying def-features --- This is an issue I noticed while reading this code for other reasons, I have only compile-tested it and I'm not sure how to test it. Christophe src/qemu/qemu_command.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6549f57..0065e83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7547,7 +7547,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; if (STREQ(def-os.arch, i686)||STREQ(def-os.arch, x86_64)) -def-features = (1 VIR_DOMAIN_FEATURE_ACPI) +def-features |= (1 VIR_DOMAIN_FEATURE_ACPI) /*| (1 VIR_DOMAIN_FEATURE_APIC)*/; #define WANT_VALUE() \ const char *val = progargv[++i]; \ Cool, so now we don't overwrite PAE feature set just a few lines above (not visible in this diff though). ACK with this squashed in: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml index e07c1f6..8abcb51 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml @@ -8,6 +8,9 @@ type arch='i686' machine='pc'hvm/type boot dev='network'/ /os + features +pae/ + /features clock offset='utc' timer name='kvmclock' present='no'/ /clock Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Intend to add OVA installation API
Ata Bohra ata.husain at hotmail.com writes: Thanks again Doug. With this direction, I have started looking into details of both formats and also to convert OVF - libvirt domain XML format. Will post the review once I get a satisfactory code to share with the community. Appreciate your support/suggestion. Thanks!Ata Date: Sun, 24 Jun 2012 17:27:03 -0500 Subject: Re: [libvirt] Intend to add OVA installation API From: cardoe at cardoe.com To: ata.husain at hotmail.com CC: libvir-list at redhat.com On Sun, Jun 24, 2012 at 5:05 PM, Ata Bohra ata.husain at hotmail.com wrote: Thanks Doug for your suggestions. I believe you are correct about the relation between OVA and OVF. But I am not 100 % possitive about your suggestion: defining an appropriate domain in libvirt. To understand better I am sharing more details about my plans: 1. Enhance libvirt interface code (libvirt.c) to provide a domain-independent routine: virDomainCreateOVA, an alternate API to create domain. To make client code real simple, this routine can take ova path as input and internally strip the OVA to extract required details. (planning to define a struct to hold all essential information). 2. Second, to enhance ESX driver to perform ESX specfic calls. Given OVA is a tar file, the parsing is just another file open/read operation; it would be simple to perform it inside domain_conf.c (infact I have written a parser to strip information off OVA already). Hope to get some comments/suggestions on above steps. Thanks! Ata Right. I'm suggesting you don't go that route and approach the problem from another angle. I did a little Googling since my last e-mail to at least make sure I understood the basics. So an OVF looks like the following: virtualappliance/package.ovf virtualappliance/disk1.vmdk virtualappliance/disk2.vmdk virtualappliance/cdrom.iso virtualappliance/en-US-resources.xml An OVA would simply be a tar of the above and named virtualappliance.ova package.ovf is an XML file containing the description of the hardware of the virtual machine, much like the XML that libvirt stores about domains. While en-US-resources.xml would be the US English descriptions of the machine and its hardware. I'm suggesting you write an application that transforms package.ovf into libvirt's own domain XML format and simply call virDomainDefineXML() rather than adding API to libvirt itself. You could then further extend the application to allow you to take a libvirt domain and export it as a OVA. Looking at VMWare and Xen, they both treat OVA/OVF as a foreign format and require a converter application to import them to their native internals so it wouldn't be much different than their approach. Just my 2 cents. -- Doug Goldstein Hi Ata Bohra, I am very happy to see this mail chain on OVF to libvirt xml conversion. I appreciate your efforts on this. Even we are looking to have this kind of conversion utility. Could you please share if you have more details on this? Thanks in advance. Best Regards Raghu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] docs: Improve patch submission guidelines
We should really advise (new) developers to send rebased patches that apply cleanly and use git-send-email rather than all other obscure ways. --- HACKING | 22 +- docs/hacking.html.in | 33 - 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/HACKING b/HACKING index 69ea96b..def94ee 100644 --- a/HACKING +++ b/HACKING @@ -21,9 +21,29 @@ or: git diff libvirt-myfeature.patch +However, the usual workflow of libvirt developer is: git checkout master + git pull + git checkout -b workbranch + Hack, committing any changes along the way + +Then, when you want to post your patches: git checkout master + git pull + git checkout workbranch + git rebase master + (fix any conflicts) + git send-email --compose --to=libvir-list@redhat.com master + +Please follow this as close as you can, especially the rebase and git +send-email part as it makes developer's, who is reviewing your patch set, life +easier. One should avoid sending patches as attachment but rather send them in +email body among with commit message. + (3) Split large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch and an explanation of how the -sequence of patches fits together. +sequence of patches fits together. Moreover, please keep in mind that it's +required to be able to compile cleanly after each patch. + + (4) Make sure your patches apply against libvirt GIT. Developers only follow GIT and don't care much about released versions. diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 89f9980..af852e0 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -11,19 +11,42 @@ lipPost patches in unified diff format. A command similar to this should work:/p -pre +delpre diff -urp libvirt.orig/ libvirt.modified/ gt; libvirt-myfeature.patch -/pre +/pre/del p or: /p pre git diff libvirt-myfeature.patch /pre - /li - liSplit large changes into a series of smaller patches, self-contained +However, the usual workflow of libvirt developer is: +pre + git checkout master + git pull + git checkout -b workbranch + Hack, committing any changes along the way +/pre +Then, when you want to post your patches: +pre + git checkout master + git pull + git checkout workbranch + git rebase master + (fix any conflicts) + git send-email --compose --no-chain-reply-to --to=libvir-list@redhat.com master +/pre +pPlease follow this as close as you can, especially the rebase and git +send-email part as it makes developer's, who is reviewing your patch +set, life easier. One should avoid sending patches as attachment but +rather send them in email body among with commit message./p/li + + lipSplit large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch and an explanation of how -the sequence of patches fits together./li +the sequence of patches fits together. Moreover, please keep in mind that +it's required to be able to compile cleanly after each patch./p + /li + liMake sure your patches apply against libvirt GIT. Developers only follow GIT and don't care much about released versions./li lipRun the automated tests on your code before submitting any changes. -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 1/8] parallels: add driver skeleton
On 07/05/12 12:58, Dmitry Guryanov wrote: Parallels Virtuozzo Server is a cloud-ready virtualization solution that allows users to simultaneously run multiple virtual machines and containers on the same physical server. Current name of this product is Parallels Server Bare Metal and more information about it can be found here - http://www.parallels.com/products/server/baremetal/sp/. This first patch adds driver, which can report node info only. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- cfg.mk |1 + configure.ac | 23 docs/drvparallels.html.in| 28 include/libvirt/virterror.h |2 +- libvirt.spec.in |9 +- mingw-libvirt.spec.in|6 + po/POTFILES.in |1 + src/Makefile.am | 13 ++ src/conf/domain_conf.c |3 +- src/conf/domain_conf.h |1 + src/driver.h |1 + src/libvirt.c|9 ++ src/parallels/parallels_driver.c | 271 ++ src/parallels/parallels_driver.h | 51 +++ src/util/virterror.c |3 +- 15 files changed, 418 insertions(+), 4 deletions(-) create mode 100644 docs/drvparallels.html.in create mode 100644 src/parallels/parallels_driver.c create mode 100644 src/parallels/parallels_driver.h @@ -2805,6 +2827,7 @@ AC_MSG_NOTICE([ LXC: $with_lxc]) AC_MSG_NOTICE([PHYP: $with_phyp]) AC_MSG_NOTICE([ ESX: $with_esx]) AC_MSG_NOTICE([ Hyper-V: $with_hyperv]) +AC_MSG_NOTICE([ PARALLELS: $with_parallels]) This line should be aligned with others. AC_MSG_NOTICE([Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) diff --git a/docs/drvparallels.html.in b/docs/drvparallels.html.in new file mode 100644 index 000..976dea1 --- /dev/null +++ b/docs/drvparallels.html.in @@ -0,0 +1,28 @@ +htmlbody +h1Parallels Virtuozzo Server driver/h1 +ul id=toc/ul +p +The libvirt PARALLELS driver can manage Parallels Virtuozzo Server starting from 6.0 version. ... from version 6.0. +/p + + +h2a name=projectProject Links/a/h2 +ul + li +The a href=http://www.parallels.com/products/server/baremetal/sp/;Parallels Virtuozzo Server/a Virtualization Solution. + /li +/ul + + +h2a name=uriConnections to the Parallels Virtuozzo Server driver/a/h2 +p +The libvirt PARALLELS driver is a single-instance privileged driver, with a driver name of 'parallels'. Some example connection URIs for the libvirt driver are: +/p +pre +parallels:///default (local access) +parallels+unix:///default(local access) +parallels://example.com/default (remote access, TLS/x509) +parallels+tcp://example.com/default (remote access, SASl/Kerberos) +parallels+ssh://r...@example.com/default (remote access, SSH tunnelled) +/pre +/body/html diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 0e0bc9c..25e8d43 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -97,7 +97,7 @@ typedef enum { VIR_FROM_URI = 45, /* Error from URI handling */ VIR_FROM_AUTH = 46, /* Error from auth handling */ VIR_FROM_DBUS = 47, /* Error from DBus */ - +VIR_FROM_PARALLELS = 48, /* Error from PARALLELS */ The comment is misaligned and the one empty line should remain here. # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST # endif diff --git a/libvirt.spec.in b/libvirt.spec.in index ec2b3b4..21d9bc0 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -67,6 +67,7 @@ %define with_esx 0%{!?_without_esx:1} %define with_hyperv0%{!?_without_hyperv:1} %define with_xenapi0%{!?_without_xenapi:1} +%define with_parallels 0%{!?_without_parallels:1} Again, misaligned. # Then the secondary host drivers, which run inside libvirtd %define with_network 0%{!?_without_network:%{server_drivers}} @@ -131,6 +132,7 @@ %define with_xenapi 0 %define with_libxl 0 %define with_hyperv 0 +%define with_parallels 0 %endif # Fedora 17 / RHEL-7 are first where we use systemd. Although earlier @@ -1032,6 +1034,10 @@ of recent versions of Linux (and other OSes). %define _without_vmware --without-vmware %endif +%if ! %{with_parallels} +%define _without_parallels --without-parallels +%endif + %if ! %{with_polkit} %define _without_polkit --without-polkit %endif @@ -1170,6 +1176,7 @@ autoreconf -if %{?_without_esx} \ %{?_without_hyperv} \ %{?_without_vmware} \ + %{?_without_parallels} \ %{?_without_network} \ %{?_with_rhel5_api} \ %{?_without_storage_fs} \ @@ -1360,7 +1367,7 @@ fi /sbin/chkconfig --add libvirtd if
Re: [libvirt] [PATCH] systemd: start libvirtd after network
Osier Yang wrote: On 2012年07月10日 03:41, Jim Fehlig wrote: Domains configured with autostart may fail to start if the host network stack has not been started. E.g. when using bridged networking autostarting a domain can fail with libvirtd[1403]: 2012-06-20 13:23:49.833+: 1485: error : qemuAutostartDomain:177 : Failed to autostart VM 'test': Cannot get interface MTU on 'br0': No such device --- daemon/libvirtd.service.in |1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 33724ee..b7afadf 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -6,6 +6,7 @@ [Unit] Description=Virtualization daemon Before=libvirt-guests.service +After=network.target [Service] EnvironmentFile=-/etc/sysconfig/libvirtd Makes sense from my p.o.v, ACK Thanks, pushed. Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix a string format bug in qemu_cgroup.c
On 07/05/2012 09:08 PM, Wen Congyang wrote: At 07/06/2012 09:53 AM, tangchen Wrote: Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/qemu/qemu_cgroup.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f8f375f..662d41d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -473,8 +473,8 @@ cleanup: rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) virReportSystemError(-rc, - _(%s), - Unable to rollback cpu bandwidth period); + %s, + _(Unable to rollback cpu bandwidth period)); } return -1; ACK I use make syntax-check , and do not find this bug... That says our syntax-check rule is not strong enough. I'll work on a patch for that, as there are other violations of this bug. -- 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] Added timestamps to storage volumes
The access, modification and change times are added to storage volumes and corresponding xml representations. --- docs/formatstorage.html.in| 13 + docs/schemas/storagevol.rng | 23 +++ src/conf/storage_conf.c | 12 src/conf/storage_conf.h |9 + src/storage/storage_backend.c | 20 5 files changed, 77 insertions(+) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d0e4319..c4d6d85 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -141,6 +141,11 @@ lt;modegt;0744lt;/modegt; lt;labelgt;virt_image_tlt;/labelgt; lt;/permissionsgt; + lt;timestampsgt; +lt;atimegt;1341933637.27319099lt;/atimegt; +lt;mtimegt;1341930622.47245868lt;/mtimegt; +lt;ctimegt;1341930622.47245868lt;/ctimegt; + lt;/timestampsgt; lt;encryption type='...'gt; ... lt;/encryptiongt; @@ -172,6 +177,14 @@ contains the MAC (eg SELinux) label string. span class=sinceSince 0.4.1/span /dd + dtcodetimestamps/code/dt + ddProvides timing information about the volume. The three sub elements +codeatime/code, codemtime/code and codectime/code hold the +access, modification and respectively the change time of the volume. The +used time format is lt;secondsgt;.lt;nanosecondsgt; since the beginning +of the epoch. This is a readonly attribute and is ignored when creating +a volume. span class=sinceSince 0.10.0/span + /dd dtcodeencryption/code/dt ddIf present, specifies how the volume is encrypted. See the a href=formatstorageencryption.htmlStorage Encryption/a page diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 7a74331..dafe918 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -63,6 +63,28 @@ /optional /define + define name='timestamps' +optional + element name='timestamps' +element name='atime' + data type=string +param name=pattern[0-9]+\.[0-9]+/param + /data +/element +element name='mtime' + data type=string +param name=pattern[0-9]+\.[0-9]+/param + /data +/element +element name='ctime' + data type=string +param name=pattern[0-9]+\.[0-9]+/param + /data +/element + /element +/optional + /define + define name='target' element name='target' optional @@ -72,6 +94,7 @@ /optional ref name='format'/ ref name='permissions'/ + ref name='timestamps'/ optional ref name='encryption'/ /optional diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ab8df9e..435ad00 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1272,6 +1272,18 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAddLit(buf,/permissions\n); +virBufferAddLit(buf, timestamps\n); +virBufferAsprintf(buf, atime%llu.%lu/atime\n, + (unsigned long long) def-timestamps.atime.tv_sec, + def-timestamps.atime.tv_nsec); +virBufferAsprintf(buf, mtime%llu.%lu/mtime\n, + (unsigned long long) def-timestamps.mtime.tv_sec, + def-timestamps.mtime.tv_nsec); +virBufferAsprintf(buf, ctime%llu.%lu/ctime\n, + (unsigned long long) def-timestamps.ctime.tv_sec, + def-timestamps.ctime.tv_nsec); +virBufferAddLit(buf, /timestamps\n); + if (def-encryption) { virBufferAdjustIndent(buf, 4); if (virStorageEncryptionFormat(buf, def-encryption) 0) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 5733b57..977b136 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -46,6 +46,14 @@ struct _virStoragePerms { /* Storage volumes */ +typedef struct _virStorageTimestamps virStorageTimestamps; +typedef virStorageTimestamps *virStorageTimestampsPtr; +struct _virStorageTimestamps { +struct timespec atime; +struct timespec mtime; +struct timespec ctime; +}; + /* * How the volume's data is stored on underlying @@ -77,6 +85,7 @@ struct _virStorageVolTarget { char *path; int format; virStoragePerms perms; +virStorageTimestamps timestamps; int type; /* only used by disk backend for partition type */ /* Currently used only in virStorageVolDef.target, not in .backingstore. */ virStorageEncryptionPtr encryption; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2e9b51..ce4d808 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1156,6 +1156,9 @@
Re: [libvirt] [PATCH] Added timestamps to storage volumes
On 07/10/2012 09:22 AM, Hendrik Schwartke wrote: The access, modification and change times are added to storage volumes and corresponding xml representations. --- docs/formatstorage.html.in| 13 + docs/schemas/storagevol.rng | 23 +++ src/conf/storage_conf.c | 12 src/conf/storage_conf.h |9 + src/storage/storage_backend.c | 20 5 files changed, 77 insertions(+) /dd + dtcodetimestamps/code/dt + ddProvides timing information about the volume. The three sub elements +codeatime/code, codemtime/code and codectime/code hold the +access, modification and respectively the change time of the volume. The Grammar: hold the access, modification, and change times of the volume, where known. no need to mention 'respectively'. +++ b/src/conf/storage_conf.c @@ -1272,6 +1272,18 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAddLit(buf,/permissions\n); +virBufferAddLit(buf, timestamps\n); +virBufferAsprintf(buf, atime%llu.%lu/atime\n, + (unsigned long long) def-timestamps.atime.tv_sec, + def-timestamps.atime.tv_nsec); Technically, tv_nsec is a signed long, and you should be using %ld instead of %lu. +++ b/src/storage/storage_backend.c @@ -1156,6 +1156,9 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, unsigned long long *capacity) { struct stat sb; +struct timespec *const atime=target-timestamps.atime, Space on either side of '='. +atime-tv_sec = sb.st_atime; +mtime-tv_sec = sb.st_mtime; +catime-tv_sec = sb.st_ctime; +#if _BSD_SOURCE || _SVID_SOURCE +atime-tv_nsec = sb.st_atim.tv_nsec; Yuck. I've nearly got consensus to use the gnulib stat-time module, in which case this would instead be the much simpler: #include stat-time.h ... atime = get_stat_atime(sb); mtime = get_stat_mtime(sb); ctime = get_stat_mtime(sb); But before we can use the gnulib stat-time module, we have to update .gnulib and bootstrap.conf; and I'm holding off on the .gnulib update until after fixed Automake is available in at least Fedora 17 (right now, unless you are manually using automake 1.11.6 or newer, you are injecting a security bug into every other package that uses automake). Also, with gnulib's stat-time module, my earlier suggestion to include birthtime would be as simple as: btime = get_state_birthtime(sb); along with filtering the display: if (btime-tv_nsec == -1) { /* birth time not available */ } -- 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] [PATCHv3 4/5] S390: Domain Schema for s390-virtio machines.
On 09.07.2012 14:33, Viktor Mihajlovski wrote: On 07/03/2012 06:18 PM, Michal Privoznik wrote: On 29.06.2012 17:02, Viktor Mihajlovski wrote: Added s390-virtio machine type to the XML schema for domains in order to not fail the domain schema tests. Signed-off-by: Viktor Mihajlovskimihaj...@linux.vnet.ibm.com --- docs/schemas/domaincommon.rng | 20 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 912a1a2..70c7d16 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -283,6 +283,7 @@ ref name=hvmsparc/ ref name=hvmppc/ ref name=hvmppc64/ +ref name=hvms390/ /choice /optional valuehvm/value @@ -369,6 +370,25 @@ /optional /group /define +define name=hvms390 +group +optional +attribute name=arch +choice +values390/value +values390x/value +/choice +/attribute +/optional +optional +attribute name=machine +choice +values390-virtio/value [1]^^ +/choice +/attribute +/optional +/group +/define define name=osexe element name=os element name=type Sorry cannot ACK this one until you update the documentation as well. Michal Hi Michal, actually I was pondering about a doc update when preparing the patches. I only wasn't clear where to put it. The only place where possible arch/machine values are mentioned seems to be in formatcaps.html.in. Would you expect me to add a sample output of the capabilities XML for s390 with some comments in there, or did you have something else in mind? Thanks. Actually, now I am going through docs I don't see a proper place neither. Moreover, in formatdomain.html.in we state: The Capabilities XML provides details on allowed values for these [these = @machine and @type] So as long as we report them in capabilities XML I guess we don't really need an doc extension. However, I think this [1] should be virtio-s390 instead of s390-virtio since we use the former among the code. What do you think? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 1/8] parallels: add driver skeleton
On 07/10/2012 06:53 PM, Peter Krempa wrote: On 07/05/12 12:58, Dmitry Guryanov wrote: Parallels Virtuozzo Server is a cloud-ready virtualization solution that allows users to simultaneously run multiple virtual machines and containers on the same physical server. Current name of this product is Parallels Server Bare Metal and +static int +parallelsGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer) +{ +/* TODO */ +*hvVer = 6; I hope this hack gets updated in a patch later on. Also this produces following output: virsh # version Compiled against library: libvir 0.9.13 Using library: libvir 0.9.13 Using API: PARALLELS 0.9.13 Running hypervisor: PARALLELS 0.0.6 Yes, I'll fix this soon. +return 0; +} + +static virDriver parallelsDriver = { +.no = VIR_DRV_PARALLELS, +.name = PARALLELS, +.open = parallelsOpen,/* 0.10.0 */ +.close = parallelsClose, /* 0.10.0 */ +.version = parallelsGetVersion, /* 0.10.0 */ +.getHostname = virGetHostname, /* 0.10.0 */ +.nodeGetInfo = nodeGetInfo, /* 0.10.0 */ +.getCapabilities = parallelsGetCapabilities, /* 0.10.0 */ +}; + +/** + * parallelsRegister: + * + * Registers the parallels driver + */ +int +parallelsRegister(void) +{ +char *prlctl_path; + +prlctl_path = virFindFileInPath(PRLCTL); +if (!prlctl_path) { +parallelsError(VIR_ERR_INTERNAL_ERROR, %s, + _(Can't find prlctl command in the PATH env)); +return VIR_DRV_OPEN_ERROR; +} Memory leak: virFindFileInPath states: /* * Finds a requested executable file in the PATH env. e.g.: * kvm-img will return /usr/bin/kvm-img * * You must free the result */ char *virFindFileInPath(const char *file) VIR_FREE(prctl_path) Also this piece of code is somewhat user-unfriendly. If you don't have the prlctl command, the driver is not registered and for some reason the error message is not present in the logs. Eric Blake suggested moving this check from parallelsOpen to parallelsRegister, http://www.redhat.com/archives/libvir-list/2012-May/msg00026.html. + +if (virRegisterDriver(parallelsDriver) 0) +return -1; + +return 0; +} diff --git a/src/parallels/parallels_driver.h b/src/parallels/parallels_driver.h new file mode 100644 index 000..c04db35 --- /dev/null +++ b/src/parallels/parallels_driver.h @@ -0,0 +1,51 @@ +/* + * parallels_driver.c: core driver functions for managing + * Parallels Virtuozzo Server hosts + * + * Copyright (C) 2012 Parallels, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef PARALLELS_DRIVER_H +# define PARALLELS_DRIVER_H + + +# include domain_conf.h +# include storage_conf.h +# include domain_event.h + +# define parallelsError(code, ...) \ +virReportErrorHelper(VIR_FROM_TEST, code, __FILE__, \ s/VIR_FROM_TEST/VIR_FROM_THIS/ + __FUNCTION__, __LINE__, __VA_ARGS__) +# define PRLCTL prlctl + + +struct _parallelsConn { +virMutex lock; +virDomainObjList domains; +virStoragePoolObjList pools; +virCapsPtr caps; +virDomainEventStatePtr domainEventState; +}; + +typedef struct _parallelsConn parallelsConn; + +typedef struct _parallelsConn *parallelsConnPtr; All of the above definitions belong to the driver .c file as we don't want to expose the internals. + +int parallelsRegister(void); + +#endif diff --git a/src/util/virterror.c b/src/util/virterror.c index cb37be0..7c0119f 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -99,7 +99,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, URI Utils, /* 45 */ Authentication Utils, - DBus Utils + DBus Utils, + Parallels Cloud Server ) I'm attaching a patch that contains my findings. I'm inclining to giving an ACK to this patch with the changes I suggested, but I'd be glad if somebody else could have a quick look. Let's see how the rest of the series works. Peter Thanks, I agree with all your comments and fixes. -- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] openvz: Handle domain obj hash map errors
This makes the driver fail with a clear error message in case of UUID collisions (for example if somebody copied a container configuration without updating the UUID) and also raises an error on other hash map failures. OpenVZ itself doesn't complain about duplicate UUIDs since this parameter is only used by libvirt. --- This version adds an extra check for hash collisions and keeps the generic failure message for all other errors in virHashAddEntry. Cheers, -- Guido src/openvz/openvz_conf.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 503c8a0..91cd125 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -684,8 +684,18 @@ int openvzLoadDomains(struct openvz_driver *driver) { openvzReadMemConf(dom-def, veid); virUUIDFormat(dom-def-uuid, uuidstr); -if (virHashAddEntry(driver-domains.objs, uuidstr, dom) 0) +if (virHashLookup(driver-domains.objs, uuidstr)) { +openvzError(VIR_ERR_INTERNAL_ERROR, +_(Duplicate container UUID %s detected for %d), +uuidstr, +veid); +goto cleanup; +} +if (virHashAddEntry(driver-domains.objs, uuidstr, dom) 0) { +openvzError(VIR_ERR_INTERNAL_ERROR, +_(Could not add UUID for container %d), veid); goto cleanup; +} virDomainObjUnlock(dom); dom = NULL; -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problem setting CPU topology
On Tue, Jul 10, 2012 at 11:54:05AM +0200, Christophe Fergeau wrote: On Sat, Jul 07, 2012 at 07:10:53PM +0300, Zeeshan Ali (Khattak) wrote: Hi, I'm trying to set exact CPU topology to qemu-kvm domains to match host's topology. In my case, host topology is: 1 socket, 2 cores and 2 threads. If I set the XML like this: domain type='kvm' .. vcpu placement='static'4/vcpu os type arch='i686' machine='pc-0.15'hvm/type boot dev='hd'/ /os cpu mode='host-model' model fallback='allow'/ topology sockets='1' cores='2' threads='2'/ /cpu .. The qemu commandline launched for this domain looks like this: /usr/bin/qemu-kvm -name fedora17-2 -S -M pc-0.15 -cpu core2duo,+lahf_lm,+rdtscp,+aes,+popcnt,+sse4.2,+sse4.1,+pdcm,+xtpr,+cx16,+tm2,+est,+smx,+vmx,+ds_cpl,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+ds -enable-kvm -m 1152 -smp 4,sockets=1,cores=2,threads=2 -uuid c573342b-2876-05b8-098e-6d5314cab062 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/home/zeenix/.config/libvirt/qemu/lib/fedora17-2.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -no-kvm-pit-reinjection -no-shutdown -no-acpi I debugged this together with Zeeshan, and the issue comes from this -no-acpi flag which libvirt added because the domain XML was missing featuresacpi//features So in the end that was a bug in Boxes, not really a libvirt/qemu issue. Unless libvirt/qemu should be taught that CPU topology won't work without ACPI and complain about it? I don't think it's really impossible for any guest OS to recognize the CPU topology or boot the other CPUs without ACPI. It looks like it's just a limitation of (most?) guest OSes. If that's the case, libvirt or QEMU can't prevent the user from running a (possibly valid) configuration just because it's not very common. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: added description of the vendor_id attribute
I know I'm late in this vendor_id stuff but it hasn't been released yet so I'm not too late. I assume the reason for introducing it is to be able to lie to a guest. Please, correct me, if this is not the case. diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94c555f..b6e0d5d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -779,7 +779,11 @@ in which case an attempt to start a domain requesting an unsupported CPU model will fail. Supported values for codefallback/code attribute are: codeallow/code (this is the default), and -codeforbid/code./dd +codeforbid/code. The optional codevendor_id/code attribute +(span class=sinceSince 0.9.14/span) can be used to set the +vendor id seen by the guest. It must be exactly 12 characters long. +If not set the vendor id of the host is used. Typical possible +values are AuthenticAMD and GenuineIntel./dd dtcodevendor/code/dt ddspan class=sinceSince 0.8.3/span the content of the This is wrong (unless your previous patch explicitly modified the code to behave like this). If vendor_id is not set, a guest should see model's default vendor. If a guest is configured with, e.g., SandyBridge model, its vendor will be GenuineIntel. If a guest uses Opteron_G3, it will see AuthenticAMD vendor. All this regardless on the vendor of the host CPU as longs as the host CPU is capable enough to support all features of a given guest CPU. Anyway, to be honest, I'm not a big fan of the new vendor_id attribute. Currently we have cpu ... model vendor_id=bleblableblaModel/model ... /cpu to force bleblablebla vendor ID on the guest CPU and cpu ... modelModel/model vendorIntel/vendor ... /cpu to make sure the guest will be run only on a host with Intel CPU. I think it would be much better to reuse the already existing vendor element. We could perhaps add new force attribute for vendor element. Thus, cpu modelModel/model vendor force='bleblablebla'/ /cpu would force bleblablebla vendor ID, cpu modelModel/model vendorIntel/vendor /cpu would just check that host CPU is made by Intel, and cpu modelModel/model vendor force='bleblablebla'Intel/vendor /cpu would check that host CPU is made by Intel but the guest CPU will have bleblablebla vendor ID. I was also thinking about making use of our vendor aliases (Intel for GenuineIntel and AMD for AuthenticAMD, and we are free to add others) but since vendor ID forcing seems to be only useful for testing, I think it's fine if we require full vendor ID to be used. At least I don't see another reason why anyone would want to confuse a guest OS by giving it Opteron_G4 CPU made by VIA. What other think about this? Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 1/8] parallels: add driver skeleton
On 07/10/12 20:50, Dmitry Guryanov wrote: On 07/10/2012 06:53 PM, Peter Krempa wrote: On 07/05/12 12:58, Dmitry Guryanov wrote: Parallels Virtuozzo Server is a cloud-ready virtualization solution that allows users to simultaneously run multiple virtual machines and containers on the same physical server. Current name of this product is Parallels Server Bare Metal and +static int +parallelsGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer) +{ +/* TODO */ +*hvVer = 6; I hope this hack gets updated in a patch later on. Also this produces following output: virsh # version Compiled against library: libvir 0.9.13 Using library: libvir 0.9.13 Using API: PARALLELS 0.9.13 Running hypervisor: PARALLELS 0.0.6 Yes, I'll fix this soon. Ok. In this case, I'm fine with a temporary solution. +return 0; +} + +static virDriver parallelsDriver = { +.no = VIR_DRV_PARALLELS, +.name = PARALLELS, +.open = parallelsOpen,/* 0.10.0 */ +.close = parallelsClose, /* 0.10.0 */ +.version = parallelsGetVersion, /* 0.10.0 */ +.getHostname = virGetHostname, /* 0.10.0 */ +.nodeGetInfo = nodeGetInfo, /* 0.10.0 */ +.getCapabilities = parallelsGetCapabilities, /* 0.10.0 */ +}; + +/** + * parallelsRegister: + * + * Registers the parallels driver + */ +int +parallelsRegister(void) +{ +char *prlctl_path; + +prlctl_path = virFindFileInPath(PRLCTL); +if (!prlctl_path) { +parallelsError(VIR_ERR_INTERNAL_ERROR, %s, + _(Can't find prlctl command in the PATH env)); +return VIR_DRV_OPEN_ERROR; +} Memory leak: virFindFileInPath states: /* * Finds a requested executable file in the PATH env. e.g.: * kvm-img will return /usr/bin/kvm-img * * You must free the result */ char *virFindFileInPath(const char *file) VIR_FREE(prctl_path) Also this piece of code is somewhat user-unfriendly. If you don't have the prlctl command, the driver is not registered and for some reason the error message is not present in the logs. Eric Blake suggested moving this check from parallelsOpen to parallelsRegister, http://www.redhat.com/archives/libvir-list/2012-May/msg00026.html. I see. Yes I agree with Eric on this, but we'll have to find out why the error message doesn't get logged. I wanted to test the driver overall and I just got rejected without any sign of what is wrong and it seemed as if the driver wasn't even compiled. I'll have a look at it while reviewing the rest of the series (that will hopefully go faster as this first patch. I'm not as skilled as Eric in reviews and I just don't want to make mistakes :) ) + +if (virRegisterDriver(parallelsDriver) 0) +return -1; + +return 0; +} diff --git a/src/parallels/parallels_driver.h b/src/parallels/parallels_driver.h new file mode 100644 index 000..c04db35 --- /dev/null +++ b/src/parallels/parallels_driver.h @@ -0,0 +1,51 @@ +/* + * parallels_driver.c: core driver functions for managing + * Parallels Virtuozzo Server hosts + * + * Copyright (C) 2012 Parallels, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef PARALLELS_DRIVER_H +# define PARALLELS_DRIVER_H + + +# include domain_conf.h +# include storage_conf.h +# include domain_event.h + +# define parallelsError(code, ...) \ +virReportErrorHelper(VIR_FROM_TEST, code, __FILE__, \ s/VIR_FROM_TEST/VIR_FROM_THIS/ + __FUNCTION__, __LINE__, __VA_ARGS__) +# define PRLCTL prlctl + + +struct _parallelsConn { +virMutex lock; +virDomainObjList domains; +virStoragePoolObjList pools; +virCapsPtr caps; +virDomainEventStatePtr domainEventState; +}; + +typedef struct _parallelsConn parallelsConn; + +typedef struct _parallelsConn *parallelsConnPtr; All of the above definitions belong to the driver .c file as we don't want to expose the internals. + +int parallelsRegister(void); + +#endif diff --git a/src/util/virterror.c b/src/util/virterror.c index cb37be0..7c0119f 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -99,7 +99,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, URI Utils, /* 45 */ Authentication Utils, -
[libvirt] [PATCH 1/5] Add virGetHostname
to query a guests's hostname. Containers like LXC and OpenVZ allow to set a hostname different from the hosts name and QEMU's guest agent could provide similar functionality. --- include/libvirt/libvirt.h.in |2 ++ src/driver.h |6 ++ src/libvirt.c| 42 ++ src/libvirt_public.syms |5 + 4 files changed, 55 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6e8d5dd..496a478 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain, int virDomainGetMaxVcpus(virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +char * virDomainGetHostname(virDomainPtr domain, + unsigned int flags); typedef enum { VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on description */ diff --git a/src/driver.h b/src/driver.h index b3c1740..46d9846 100644 --- a/src/driver.h +++ b/src/driver.h @@ -142,6 +142,11 @@ typedef int unsigned int flags); typedef char * (*virDrvDomainGetOSType)(virDomainPtr domain); + +typedef char * +(*virDrvDomainGetHostname) (virDomainPtr domain, + unsigned int flags); + typedef unsigned long long (*virDrvDomainGetMaxMemory) (virDomainPtr domain); typedef int @@ -904,6 +909,7 @@ struct _virDriver { virDrvDomainDestroy domainDestroy; virDrvDomainDestroyFlagsdomainDestroyFlags; virDrvDomainGetOSType domainGetOSType; +virDrvDomainGetHostname domainGetHostname; virDrvDomainGetMaxMemorydomainGetMaxMemory; virDrvDomainSetMaxMemorydomainSetMaxMemory; virDrvDomainSetMemory domainSetMemory; diff --git a/src/libvirt.c b/src/libvirt.c index db6ba15..563482e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18973,3 +18973,45 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainGetHostname: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Get the hostname for that domain + * + * + * Returns a pointer to the name or NULL. + */ +char * +virDomainGetHostname(virDomainPtr domain, unsigned int flags) +{ +virConnectPtr conn; + +VIR_DEBUG(domain=%p, domain); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN(domain)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return NULL; +} + +conn = domain-conn; + +if (conn-driver-domainGetHostname) { +char *ret; +ret = conn-driver-domainGetHostname (domain, flags); +if (!ret) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(domain-conn); +return NULL; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..1a8e58a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -544,4 +544,9 @@ LIBVIRT_0.9.13 { virDomainSnapshotRef; } LIBVIRT_0.9.11; +LIBVIRT_0.9.14 { +global: +virDomainGetHostname; +} LIBVIRT_0.9.13; + # define new API here using predicted next version number -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] virsh: allow to print hostname in domain listings
--- tools/virsh.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {managed-save, VSH_OT_BOOL, 0, N_(mark inactive domains with managed save state)}, {title, VSH_OT_BOOL, 0, N_(show short domain description)}, +{hostname, VSH_OT_BOOL, 0, N_(show domain hostname)}, {NULL, 0, 0, NULL} }; @@ -1307,8 +1308,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) bool optTable = vshCommandOptBool(cmd, table); bool optUUID = vshCommandOptBool(cmd, uuid); bool optName = vshCommandOptBool(cmd, name); +bool optHostname = vshCommandOptBool(cmd, hostname); int i; -char *title; +char *title, *hostname; char uuid[VIR_UUID_STRING_BUFLEN]; int state; bool ret = false; @@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _(Id), _(Name), _(State), _(Title), - -); +else if (optHostname) +vshPrintExtra(ctl, %-5s %-30s %-10s %-20s\n%s\n, + _(Id), _(Name), _(State), _(Hostname), + - + -); else vshPrintExtra(ctl, %-5s %-30s %s\n%s\n, _(Id), _(Name), _(State), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title); VIR_FREE(title); +} else if (optHostname) { +if (!(hostname = virDomainGetHostname (dom, 0))) +goto cleanup; + +vshPrint(ctl, %-5s %-30s %-10s %-20s\n, id_buf, + virDomainGetName(dom), + state == -2 ? _(saved) : _(vshDomainStateToString(state)), + hostname); +VIR_FREE(hostname); } else { vshPrint(ctl, %-5s %-30s %s\n, id_buf, virDomainGetName(dom), -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] virsh: Add domhostname
to query the guest's hostname. --- tools/virsh.c | 44 tools/virsh.pod |4 2 files changed, 48 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 85b1185..591a1ce 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14131,6 +14131,49 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd) } /* + * domhostname command + */ +static const vshCmdInfo info_domhostname[] = { +{help, N_(print the domain's hostname)}, +{desc, }, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_domhostname[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdDomHostname (vshControl *ctl, const vshCmd *cmd) +{ +char *hostname; +virDomainPtr dom; +bool ret = false; + +if (!vshConnectionUsability(ctl, ctl-conn)) +return false; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +hostname = virDomainGetHostname (dom, 0); +if (hostname == NULL) { +vshError(ctl, %s, _(failed to get hostname)); +goto error; +} + +vshPrint (ctl, %s\n, hostname); +ret = true; + +error: +VIR_FREE(hostname); +virDomainFree(dom); +return ret; +} + + +/* * attach-device command */ static const vshCmdInfo info_attach_device[] = { @@ -18169,6 +18212,7 @@ static const vshCmdDef domManagementCmds[] = { {detach-interface, cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, {domdisplay, cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, +{domhostname, cmdDomHostname, opts_domhostname, info_domhostname, 0}, {domid, cmdDomid, opts_domid, info_domid, 0}, {domif-setlink, cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {domiftune, cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 93fdac7..93ced24 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -826,6 +826,10 @@ Output a URI which can be used to connect to the graphical display of the domain via VNC, SPICE or RDP. If I--include-password is specified, the SPICE channel password will be included in the URI. +=item Bdomhostname Idomain-id + +Returns the hostname of a domain. + =item Bdominfo Idomain-id Returns basic information about the domain. -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] openvz: Add openvzVEGetStringParam
to retrieve a VEs config parameters as a single string. This will be used by the upcoming domainGetHostname implementation. --- src/libvirt_openvz.syms |2 +- src/openvz/openvz_util.c | 31 +++ src/openvz/openvz_util.h |1 + 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/libvirt_openvz.syms b/src/libvirt_openvz.syms index 11c5587..1993eb5 100644 --- a/src/libvirt_openvz.syms +++ b/src/libvirt_openvz.syms @@ -1,7 +1,7 @@ # # These symbols are dependent upon --with-openvz via WITH_OPENVZ # - openvzReadConfigParam; openvzReadNetworkConf; openvzLocateConfFile; +openvzVEGetStringParam; diff --git a/src/openvz/openvz_util.c b/src/openvz/openvz_util.c index 61b55de..b172104 100644 --- a/src/openvz/openvz_util.c +++ b/src/openvz/openvz_util.c @@ -26,6 +26,9 @@ #include internal.h #include virterror_internal.h +#include command.h +#include datatypes.h +#include memory.h #include openvz_conf.h #include openvz_util.h @@ -49,3 +52,31 @@ openvzKBPerPages(void) } return kb_per_pages; } + +char* +openvzVEGetStringParam(virDomainPtr domain, const char* param) +{ +int status, len; +char *output = NULL; + +virCommandPtr cmd = virCommandNewArgList(VZLIST, + -o, + param, + domain-name, + -H , NULL); + +virCommandSetOutputBuffer(cmd, output); +if (virCommandRun(cmd, status)) { +openvzError(VIR_ERR_OPERATION_FAILED, +_(Failed to get %s for %s: %d), param, domain-name, +status); +VIR_FREE(output); +} +len = strlen(output); +/* delete trailing newline */ +if (len) +output[len-1] = '\0'; + +virCommandFree(cmd); +return output; +} diff --git a/src/openvz/openvz_util.h b/src/openvz/openvz_util.h index a0d9bbb..6a991f3 100644 --- a/src/openvz/openvz_util.h +++ b/src/openvz/openvz_util.h @@ -24,5 +24,6 @@ # define OPENVZ_UTIL_H long openvzKBPerPages(void); +char* openvzVEGetStringParam(virDomainPtr dom, const char *param); #endif -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Allow to query a guest's hostname
The following patches allow to query a guest's hostname. The last patch might be debatable since it adds yet another option to virsh list but I hope the rest of the serious looks sane. Cheers, -- Guido Guido Günther (5): Add virGetHostname virsh: Add domhostname openvz: Add openvzVEGetStringParam openvz: Implement domainGetHostname virsh: allow to print hostname in domain listings include/libvirt/libvirt.h.in |2 ++ src/driver.h |6 src/libvirt.c| 42 src/libvirt_openvz.syms |2 +- src/libvirt_public.syms |5 src/openvz/openvz_driver.c | 28 +++ src/openvz/openvz_util.c | 31 + src/openvz/openvz_util.h |1 + tools/virsh.c| 62 +- tools/virsh.pod |4 +++ 10 files changed, 181 insertions(+), 2 deletions(-) -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] openvz: Implement domainGetHostname
If the container doesn't have the hostname parameter set an empty string () is returned. --- src/openvz/openvz_driver.c | 28 1 file changed, 28 insertions(+) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index c9150e0..469d043 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -234,6 +234,33 @@ cleanup: } +static char* openvzDomainGetHostname(virDomainPtr dom, unsigned int flags) +{ +char *hostname = NULL; + +virCheckFlags(0, NULL); + +hostname = openvzVEGetStringParam(dom, hostname); +if (hostname == NULL) +goto cleanup; + +/* vzlist prints an unset hostname as '-' */ +if (strlen(hostname) == 1 hostname[0] == '-') { +VIR_FREE(hostname); +hostname = strdup(); +if (hostname == NULL) { +virReportOOMError(); +goto cleanup; +} +} +return hostname; + +cleanup: + VIR_FREE(hostname); + return NULL; +} + + static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, int id) { struct openvz_driver *driver = conn-privateData; @@ -2127,6 +2154,7 @@ static virDriver openvzDriver = { .domainIsUpdated = openvzDomainIsUpdated, /* 0.8.6 */ .isAlive = openvzIsAlive, /* 0.9.8 */ .domainUpdateDeviceFlags = openvzDomainUpdateDeviceFlags, /* 0.9.13 */ +.domainGetHostname = openvzDomainGetHostname, /* 0.9.14 */ }; int openvzRegister(void) { -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] Add virGetHostname
In the subject: s/virGetHostname/virDomainGetHostname/ On 07/10/2012 02:46 PM, Guido Günther wrote: to query a guests's hostname. Containers like LXC and OpenVZ allow to set a hostname different from the hosts name and QEMU's guest agent could provide similar functionality. --- include/libvirt/libvirt.h.in |2 ++ src/driver.h |6 ++ src/libvirt.c| 42 ++ src/libvirt_public.syms |5 + 4 files changed, 55 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6e8d5dd..496a478 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain, int virDomainGetMaxVcpus(virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +char * virDomainGetHostname(virDomainPtr domain, + unsigned int flags); Seems like it might be useful. + +/** + * virDomainGetHostname: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Get the hostname for that domain Missing an ending '.'. Maybe also mention that for some hypervisors, the guest may need to be running a guest agent. + * + * Extra blank line. + * Returns a pointer to the name or NULL. Mention that caller must free the result. + */ +char * +virDomainGetHostname(virDomainPtr domain, unsigned int flags) +{ +virConnectPtr conn; + +VIR_DEBUG(domain=%p, domain); Better to use: VIR_DOMAIN_DEBUG(domain) +++ b/src/libvirt_public.syms @@ -544,4 +544,9 @@ LIBVIRT_0.9.13 { virDomainSnapshotRef; } LIBVIRT_0.9.11; +LIBVIRT_0.9.14 { We are considering naming the next release 0.10.0; but this can be fixed up globally when we solidify the naming decision. +global: +virDomainGetHostname; +} LIBVIRT_0.9.13; + # define new API here using predicted next version number -- 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 5/5] virsh: allow to print hostname in domain listings
On 07/10/12 22:46, Guido Günther wrote: --- tools/virsh.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {managed-save, VSH_OT_BOOL, 0, N_(mark inactive domains with managed save state)}, {title, VSH_OT_BOOL, 0, N_(show short domain description)}, +{hostname, VSH_OT_BOOL, 0, N_(show domain hostname)}, {NULL, 0, 0, NULL} }; @@ -1307,8 +1308,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) bool optTable = vshCommandOptBool(cmd, table); bool optUUID = vshCommandOptBool(cmd, uuid); bool optName = vshCommandOptBool(cmd, name); +bool optHostname = vshCommandOptBool(cmd, hostname); int i; -char *title; +char *title, *hostname; char uuid[VIR_UUID_STRING_BUFLEN]; int state; bool ret = false; @@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _(Id), _(Name), _(State), _(Title), - -); +else if (optHostname) +vshPrintExtra(ctl, %-5s %-30s %-10s %-20s\n%s\n, + _(Id), _(Name), _(State), _(Hostname), + - + -); else vshPrintExtra(ctl, %-5s %-30s %s\n%s\n, _(Id), _(Name), _(State), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title); VIR_FREE(title); +} else if (optHostname) { +if (!(hostname = virDomainGetHostname (dom, 0))) +goto cleanup; + +vshPrint(ctl, %-5s %-30s %-10s %-20s\n, id_buf, + virDomainGetName(dom), + state == -2 ? _(saved) : _(vshDomainStateToString(state)), + hostname); +VIR_FREE(hostname); } else { vshPrint(ctl, %-5s %-30s %s\n, id_buf, virDomainGetName(dom), This approach has a problem: If you specify list --title --hostname you'll get just the title. When I was adding the Title option I figured that this situation would happen sooner or later. I thought about a universal table printer that could dynamicaly print desired columns, but that seemed a bit of overkill at that time. A quick option would be to make those option mutually exclusive, but that's not elegant. Also this patch misses adding docs for the new flag. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] virsh: Add domhostname
On 07/10/2012 02:46 PM, Guido Günther wrote: to query the guest's hostname. --- tools/virsh.c | 44 tools/virsh.pod |4 2 files changed, 48 insertions(+) + +static bool +cmdDomHostname (vshControl *ctl, const vshCmd *cmd) Style: no space before function (), here... +{ +char *hostname; +virDomainPtr dom; +bool ret = false; + +if (!vshConnectionUsability(ctl, ctl-conn)) +return false; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +hostname = virDomainGetHostname (dom, 0); here... +if (hostname == NULL) { +vshError(ctl, %s, _(failed to get hostname)); +goto error; +} + +vshPrint (ctl, %s\n, hostname); and here. Other than that, looks reasonable. -- 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 3/5] openvz: Add openvzVEGetStringParam
On 07/10/2012 02:46 PM, Guido Günther wrote: +char* +openvzVEGetStringParam(virDomainPtr domain, const char* param) +{ +int status, len; +char *output = NULL; + +virCommandPtr cmd = virCommandNewArgList(VZLIST, + -o, + param, + domain-name, + -H , NULL); + +virCommandSetOutputBuffer(cmd, output); +if (virCommandRun(cmd, status)) { This should check for virCommandRun() 0. Also, virCommandRun() does not guarantee that 'status' is populated unless it returns 0; if it returns -1, you may be left with outputting uninitilized data (maybe we could argue that it is a bug in virCommandRun that should be fixed...). Outputting the raw exit status isn't very handy; I think it would be simpler to just pass NULL for the second argument (which lets virCommandRun issue a reasonable error message about any failure to run the command, including the entire command line attempted, which already includes param and domain-name). +openvzError(VIR_ERR_OPERATION_FAILED, +_(Failed to get %s for %s: %d), param, domain-name, +status); +VIR_FREE(output); +} +len = strlen(output); Crash-tastic! If the command fails, then you end up calling strlen(NULL). Are you missing a goto or early return? +/* delete trailing newline */ +if (len) +output[len-1] = '\0'; Shouldn't you at least check that output[len-1] is '\n' before nuking it? Also, our style prefers spaces on either side of '-'. -- 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 4/5] openvz: Implement domainGetHostname
On 07/10/2012 02:46 PM, Guido Günther wrote: If the container doesn't have the hostname parameter set an empty string () is returned. I'd almost rather return NULL and an error code (either invent a new error, or maybe reuse VIR_ERR_OPERATION_FAILED) than an empty string if we fail to get the information. +static char* openvzDomainGetHostname(virDomainPtr dom, unsigned int flags) +{ +char *hostname = NULL; + +virCheckFlags(0, NULL); + +hostname = openvzVEGetStringParam(dom, hostname); +if (hostname == NULL) +goto cleanup; + +/* vzlist prints an unset hostname as '-' */ +if (strlen(hostname) == 1 hostname[0] == '-') { More efficient to write: if (STREQ(hostname, -)) { @@ -2127,6 +2154,7 @@ static virDriver openvzDriver = { .domainIsUpdated = openvzDomainIsUpdated, /* 0.8.6 */ .isAlive = openvzIsAlive, /* 0.9.8 */ .domainUpdateDeviceFlags = openvzDomainUpdateDeviceFlags, /* 0.9.13 */ +.domainGetHostname = openvzDomainGetHostname, /* 0.9.14 */ Again, a global version cleanup would fix this to 0.10.0. -- 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 5/5] virsh: allow to print hostname in domain listings
On 07/10/2012 02:46 PM, Guido Günther wrote: --- tools/virsh.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {managed-save, VSH_OT_BOOL, 0, N_(mark inactive domains with managed save state)}, {title, VSH_OT_BOOL, 0, N_(show short domain description)}, +{hostname, VSH_OT_BOOL, 0, N_(show domain hostname)}, {NULL, 0, 0, NULL} I'm worried that we're trying to cram too much into 'list'. Would 'dominfo' be a better fit for this type of information, without needing a command line flag? @@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _(Id), _(Name), _(State), _(Title), - -); +else if (optHostname) +vshPrintExtra(ctl, %-5s %-30s %-10s %-20s\n%s\n, + _(Id), _(Name), _(State), _(Hostname), + - + -); If we're going to support this in 'list', then we really need to implement a more generic way to format data into columns, with as many columns as needed per row. else vshPrintExtra(ctl, %-5s %-30s %s\n%s\n, _(Id), _(Name), _(State), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title); VIR_FREE(title); +} else if (optHostname) { +if (!(hostname = virDomainGetHostname (dom, 0))) No space before () in function calls. -- 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 0/5] Allow to query a guest's hostname
On 07/10/2012 02:45 PM, Guido Günther wrote: The following patches allow to query a guest's hostname. The last patch might be debatable since it adds yet another option to virsh list but I hope the rest of the serious looks sane. Cheers, -- Guido Guido Günther (5): Add virGetHostname virsh: Add domhostname openvz: Add openvzVEGetStringParam openvz: Implement domainGetHostname virsh: allow to print hostname in domain listings Missing a patch to implement the remote protocol glue for this API. Even if openvz doesn't use it, and even if we don't code up a qemu or lxc implementation at this time, the rpc code should be pretty easy to add; having it now will let an early client (such as virsh) automatically be able to use the API when a later server finally implements it. -- 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] build: detect all improper uses of _(%s)
The only useful translation of %s as a format string is %s (I suppose you could claim %$1s is also valid, but why bother). So it is not worth translating; fixing this exposes some instances where we were failing to translate real error messages. * cfg.mk (sc_prohibit_useless_translation): New rule. * src/lxc/lxc_driver.c (lxcSetVcpuBWLive): Fix offender. * src/openvz/openvz_conf.c (openvzReadFSConf): Likewise. * src/qemu/qemu_cgroup.c (qemuSetupCgroupVcpuBW) (qemuSetupCgroupForVcpu): Likewise. * src/qemu/qemu_driver.c (qemuSetVcpusBWLive): Likewise. * src/xenapi/xenapi_utils.c (xenapiSessionErrorHandle): Likewise. --- This is a followup to: https://www.redhat.com/archives/libvir-list/2012-July/msg00403.html cfg.mk|6 ++ src/lxc/lxc_driver.c |5 ++--- src/openvz/openvz_conf.c |5 ++--- src/qemu/qemu_cgroup.c| 12 +--- src/qemu/qemu_driver.c|5 ++--- src/xenapi/xenapi_utils.c |8 +--- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/cfg.mk b/cfg.mk index 7664d5d..6dfe799 100644 --- a/cfg.mk +++ b/cfg.mk @@ -622,6 +622,12 @@ sc_prohibit_newline_at_end_of_diagnostic: { echo '$(ME): newline at end of message(s)' 12; \ exit 1; } || : +# The strings and %s should never be marked for translation. +sc_prohibit_useless_translation: + @prohibit='_\((%s)?\)'\ + halt='$(ME): found useless translation' \ + $(_sc_search_regexp) + # Enforce recommended preprocessor indentation style. sc_preprocessor_indentation: @if cppi --version /dev/null 21; then\ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b58aeae..ae024fe 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2965,9 +2965,8 @@ cleanup: if (period) { rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) -virReportSystemError(-rc, - _(%s), - Unable to rollback cpu bandwidth period); +virReportSystemError(-rc, %s, + _(Unable to rollback cpu bandwidth period)); } return -1; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index ad27d37..7b66299 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -457,9 +457,8 @@ openvzReadFSConf(virDomainDefPtr def, /* Ensure that we can multiply by 1024 without overflowing. */ if (barrier ULLONG_MAX / 1024 || limit ULLONG_MAX / 1024 ) { -virReportSystemError(VIR_ERR_OVERFLOW, - _(%s), - Unable to parse quota); +virReportSystemError(VIR_ERR_OVERFLOW, %s, + _(Unable to parse quota)); goto error; } fs-space_soft_limit = barrier * 1024; /* unit is bytes */ diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f8f375f..e39f5e1 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1,7 +1,7 @@ /* * qemu_cgroup.c: QEMU cgroup management * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -472,9 +472,8 @@ cleanup: if (period) { rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) -virReportSystemError(-rc, - _(%s), - Unable to rollback cpu bandwidth period); +virReportSystemError(-rc, %s, + _(Unable to rollback cpu bandwidth period)); } return -1; @@ -507,9 +506,8 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { /* Ensure that we can multiply by vcpus without overflowing. */ if (quota LLONG_MAX / vm-def-vcpus) { -virReportSystemError(EINVAL, - _(%s), - Unable to set cpu bandwidth quota); +virReportSystemError(EINVAL, %s, + _(Unable to set cpu bandwidth quota)); goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3410535..377be9f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7134,9 +7134,8 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, /* Ensure that we can multiply by vcpus without overflowing. */ if (quota LLONG_MAX / vm-def-vcpus) { -virReportSystemError(EINVAL, - _(%s), -
Re: [libvirt] [PATCH] build: detect all improper uses of _(%s)
On 07/10/12 23:38, Eric Blake wrote: The only useful translation of %s as a format string is %s (I suppose you could claim %$1s is also valid, but why bother). So it is not worth translating; fixing this exposes some instances where we were failing to translate real error messages. * cfg.mk (sc_prohibit_useless_translation): New rule. * src/lxc/lxc_driver.c (lxcSetVcpuBWLive): Fix offender. * src/openvz/openvz_conf.c (openvzReadFSConf): Likewise. * src/qemu/qemu_cgroup.c (qemuSetupCgroupVcpuBW) (qemuSetupCgroupForVcpu): Likewise. * src/qemu/qemu_driver.c (qemuSetVcpusBWLive): Likewise. * src/xenapi/xenapi_utils.c (xenapiSessionErrorHandle): Likewise. --- This is a followup to: https://www.redhat.com/archives/libvir-list/2012-July/msg00403.html cfg.mk|6 ++ src/lxc/lxc_driver.c |5 ++--- src/openvz/openvz_conf.c |5 ++--- src/qemu/qemu_cgroup.c| 12 +--- src/qemu/qemu_driver.c|5 ++--- src/xenapi/xenapi_utils.c |8 +--- 6 files changed, 22 insertions(+), 19 deletions(-) ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: detect all improper uses of _(%s)
On 07/10/2012 03:51 PM, Peter Krempa wrote: On 07/10/12 23:38, Eric Blake wrote: The only useful translation of %s as a format string is %s (I suppose you could claim %$1s is also valid, but why bother). So %1$s, actually. ACK. Thanks; pushed with corrected commit message. -- 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] [PATCHv2] docs: Improve patch submission guidelines
On 07/10/2012 08:32 AM, Michal Privoznik wrote: We should really advise (new) developers to send rebased patches that apply cleanly and use git-send-email rather than all other obscure ways. --- HACKING | 22 +- docs/hacking.html.in | 33 - 2 files changed, 49 insertions(+), 6 deletions(-) I kind of reviewed the generated output, although obviously any changes would be to the original source html.in file. diff --git a/HACKING b/HACKING index 69ea96b..def94ee 100644 --- a/HACKING +++ b/HACKING @@ -21,9 +21,29 @@ or: git diff libvirt-myfeature.patch +However, the usual workflow of libvirt developer is: git checkout master Hmm, I would have expected a new line here. Wonder what it is about our xslt conversion that merged these, and if there is anything we can do in the .in file to force the line break? + git pull + git checkout -b workbranch + Hack, committing any changes along the way + +Then, when you want to post your patches: git checkout master + git pull + git checkout workbranch + git rebase master These can be shortened; if you do: git checkout -t origin -b workbranch to originally create your workbranch, then preparing to post is as simple as: git pull --rebase rather than jumping through two more checkouts. + (fix any conflicts) + git send-email --compose --to=libvir-list@redhat.com master I actually like 'send-email --cover-letter --annotate' better than 'send-email --compose'. I also recommend doing: git config sendemail.to libvir-list@redhat.com so that you don't have to type the --to=... designation every time. Maybe also mention that a single patch doesn't need a cover letter, but a series of 2 or more does. + +Please follow this as close as you can, especially the rebase and git +send-email part as it makes developer's, who is reviewing your patch set, life Awkward read. How about: especially the rebase and git send-email part, as it makes life easier for other developers to review your patch set. +easier. One should avoid sending patches as attachment but rather send them in s/attachment/attachments,/ +email body among with commit message. Mention that patch versioning information (such as a resend of a series to address review comments from the first version) should occur after a '---' line in the email, so that it helps reviewers but doesn't become part of git history. Also mention that 'git send-email --subject-prefix=PATCHv2' can be useful for annotating revised patches. + (3) Split large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch and an explanation of how the -sequence of patches fits together. +sequence of patches fits together. Moreover, please keep in mind that it's +required to be able to compile cleanly after each patch. good, but I'd also add: A feature does not have to work until the end of a series, as long as intermediate patches don't cause test-suite failures. + Hack, committing any changes along the way +/pre +Then, when you want to post your patches: +pre Per my above comments, this may need some p markup. -- 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] [PATCHv2] openvz: Handle domain obj hash map errors
On 07/10/2012 12:54 PM, Guido Günther wrote: This makes the driver fail with a clear error message in case of UUID collisions (for example if somebody copied a container configuration without updating the UUID) and also raises an error on other hash map failures. OpenVZ itself doesn't complain about duplicate UUIDs since this parameter is only used by libvirt. --- This version adds an extra check for hash collisions and keeps the generic failure message for all other errors in virHashAddEntry. Cheers, -- Guido 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 01/13] Introduce the function virCgroupForHypervisor
hi~ On 07/10/2012 08:44 PM, Eric Blake wrote: The subject should include 'v2' to mention that this is a respin; you can do that with 'git send-email --subject-prefix=PATCHv2'. On 07/10/2012 03:11 AM, tangchen wrote: Introduce the function virCgroupForHypervisor() to create sub directory for hypervisor thread(include I/O thread, vhost-net thread) So if I understand correctly, the intent is to create this hierarchy: cgroup mount point(s) (top-level controllers) - libvirt (subdirectory for all libvirt cgroups) --- qemu (all groups tied to a driver, virCgroupForDriver) - $name (all groups tied to a VM, virCgroupForDomain) --- $vcpuN (one group per vcpu, virCgroupForVcpu) --- hypervisor (one catchall group for non-vcpu, virCgroupForHypervisor) where hypervisor and vcpuN directories are siblings, both adding up to the sum total of $name for the VM totals. Yes, that's right. :) If so, please document that in the commit message. OK, I'll do it soon. :) Please have a look at the other patches. If there is any other problems, please let me know, and I'll fix them all in v3 patch. BTW, I'm not in a hurry. So please take your time. :) Thanks.:) Signed-off-by: Wen Congyang we...@cn.fujitsu.com Still doesn't have the correct git authorship; git send-email would automatically insert a line: From: Wen Congyang we...@cn.fujitsu.com if you had correctly attributed authorship. I can fix that as needed, though. --- .gnulib |2 +- Oops; gnulib submodule updates should generally be an independent patch, it looks like it was an accident that you bumped it on this patch. OK, I got it and I'll fix it, thanks. :) Other than that, this patch looks okay. -- Best Regards, Tang chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Problem setting CPU topology
On Tue, Jul 10, 2012 at 2:02 PM, Eduardo Habkost ehabk...@redhat.com wrote: On Tue, Jul 10, 2012 at 11:54:05AM +0200, Christophe Fergeau wrote: On Sat, Jul 07, 2012 at 07:10:53PM +0300, Zeeshan Ali (Khattak) wrote: Hi, I'm trying to set exact CPU topology to qemu-kvm domains to match host's topology. In my case, host topology is: 1 socket, 2 cores and 2 threads. If I set the XML like this: domain type='kvm' .. vcpu placement='static'4/vcpu os type arch='i686' machine='pc-0.15'hvm/type boot dev='hd'/ /os cpu mode='host-model' model fallback='allow'/ topology sockets='1' cores='2' threads='2'/ /cpu .. The qemu commandline launched for this domain looks like this: /usr/bin/qemu-kvm -name fedora17-2 -S -M pc-0.15 -cpu core2duo,+lahf_lm,+rdtscp,+aes,+popcnt,+sse4.2,+sse4.1,+pdcm,+xtpr,+cx16,+tm2,+est,+smx,+vmx,+ds_cpl,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+ds -enable-kvm -m 1152 -smp 4,sockets=1,cores=2,threads=2 -uuid c573342b-2876-05b8-098e-6d5314cab062 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/home/zeenix/.config/libvirt/qemu/lib/fedora17-2.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -no-kvm-pit-reinjection -no-shutdown -no-acpi I debugged this together with Zeeshan, and the issue comes from this -no-acpi flag which libvirt added because the domain XML was missing featuresacpi//features So in the end that was a bug in Boxes, not really a libvirt/qemu issue. Unless libvirt/qemu should be taught that CPU topology won't work without ACPI and complain about it? I don't think it's really impossible for any guest OS to recognize the CPU topology or boot the other CPUs without ACPI. It looks like it's just a limitation of (most?) guest OSes. If that's the case, libvirt or QEMU can't prevent the user from running a (possibly valid) configuration just because it's not very common. -- Eduardo Isn't MPS and APIC support required for a guest OS to really handle the topology correctly? Now days ACPI provides most of that information so its likely that QEMU doesn't support plain old MPS + APIC. Its also possible that QEMU does support this but the kernel got confused because I see the CPUID +acpi specified in the command line while -no-acpi is on the command line as you noted. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix a string format bug in qemu_cgroup.c
于 2012年07月10日 23:24, Eric Blake 写道: On 07/05/2012 09:08 PM, Wen Congyang wrote: At 07/06/2012 09:53 AM, tangchen Wrote: Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/qemu/qemu_cgroup.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f8f375f..662d41d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -473,8 +473,8 @@ cleanup: rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) virReportSystemError(-rc, - _(%s), - Unable to rollback cpu bandwidth period); + %s, + _(Unable to rollback cpu bandwidth period)); } return -1; ACK I use make syntax-check , and do not find this bug... That says our syntax-check rule is not strong enough. I'll work on a patch for that, as there are other violations of this bug. We have the same problem in lxcSetVcpuBWLive. cleanup: if (period) { rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) virReportSystemError(-rc, _(%s), Unable to rollback cpu bandwidth period); } it seems copied from qemuSetupCgroupVcpuBW. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix a string format bug in qemu_cgroup.c
At 07/11/2012 09:59 AM, Gao feng Wrote: 于 2012年07月10日 23:24, Eric Blake 写道: On 07/05/2012 09:08 PM, Wen Congyang wrote: At 07/06/2012 09:53 AM, tangchen Wrote: Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/qemu/qemu_cgroup.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f8f375f..662d41d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -473,8 +473,8 @@ cleanup: rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) virReportSystemError(-rc, - _(%s), - Unable to rollback cpu bandwidth period); + %s, + _(Unable to rollback cpu bandwidth period)); } return -1; ACK I use make syntax-check , and do not find this bug... That says our syntax-check rule is not strong enough. I'll work on a patch for that, as there are other violations of this bug. It seems that this rule does not work for multiple lines... We have the same problem in lxcSetVcpuBWLive. cleanup: if (period) { rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) virReportSystemError(-rc, _(%s), Unable to rollback cpu bandwidth period); } it seems copied from qemuSetupCgroupVcpuBW. I think so. This bug is first introduced by me. :( Thanks Wen Congyang -- 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] dnsmasq didn't work as except
On 07/10/2012 09:55 PM, ta...@linux.vnet.ibm.com wrote: hi all The vm created by libvirtd can not acquire a ip from dnsmasq. nobody 14203 1 0 21:31 ?00:00:00 /usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --conf-file= --except-interface lo --listen-address 192.168.122.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override note that --conf-file= is empty ,is there any thing I missing? Nothing is missing. what does virsh net-list, virsh net-dumpxml default and virsh dumpxml your guest say? please make sure your guest is using 'default' network. In addition, please show your libvirt, kvm and dnsmasq verstion, thanks. thanks Eli. -- 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] dnsmasq didn't work as except
On 07/10/2012 09:55 PM, ta...@linux.vnet.ibm.com wrote: hi all The vm created by libvirtd can not acquire a ip from dnsmasq. nobody 14203 1 0 21:31 ?00:00:00 /usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --conf-file= --except-interface lo --listen-address 192.168.122.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override note that --conf-file= is empty ,is there any thing I missing? Options for dnsmasq can be set either on the command line when starting dnsmasq, or in its configuration file, /etc/dnsmasq.conf(if it exists) --config-file=empty means explicitly not to accept any configure file. what kind of OS tht vm is. The possible way for vm to do is to release old lease and get a new lease from dnsmasq again linux guest: dhclient -r, then dhclient ethX windows guest: ipconfig /release, then ipconfig /renew Guannan Ren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [glib PATCH] Add givr_domain_save_to_file() and gvir_domain_save_to_file_async
--- libvirt-gobject/libvirt-gobject-domain.c | 138 ++ libvirt-gobject/libvirt-gobject-domain.h | 19 libvirt-gobject/libvirt-gobject.sym |3 + 3 files changed, 160 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 088cd33..356b15b 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -55,6 +55,7 @@ enum { VIR_RESUMED, VIR_STOPPED, VIR_UPDATED, +VIR_SAVED_TO_FILE, LAST_SIGNAL }; @@ -225,6 +226,16 @@ static void gvir_domain_class_init(GVirDomainClass *klass) G_TYPE_NONE, 0); + signals[VIR_SAVED_TO_FILE] = g_signal_new(saved_to_file, + G_OBJECT_CLASS_TYPE(object_class), + G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | + G_SIGNAL_NO_HOOKS | G_SIGNAL_DETAILED, + G_STRUCT_OFFSET(GVirDomainClass, saved_to_file), + NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, + 0); + g_type_class_add_private(klass, sizeof(GVirDomainPrivate)); } @@ -556,6 +567,133 @@ gboolean gvir_domain_reboot(GVirDomain *dom, return TRUE; } +gboolean gvir_domain_save_to_file_config(GVirDomain *dom, + gchar *filename, + GVirConfigDomain *conf, + guint flags, + GError **err) +{ +GVirDomainPrivate *priv; +gchar *custom_xml; +int ret; + +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN (conf), FALSE); +g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + +priv = dom-priv; +custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + +if (flags || custom_xml) { +ret = virDomainSaveFlags(priv-handle, filename, custom_xml, flags); +g_free (custom_xml); +} +else { +ret = virDomainSave(priv-handle, filename); +g_free (custom_xml); +} +if (ret 0) { +gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + Unable to save domain to file); +return FALSE; +} + +return TRUE; +} + +typedef struct { +gchar *filename; +gchar *custom_xml; +guint flags; +} DomainSaveToFileData; + +static void domain_save_to_file_data_free(DomainSaveToFileData *data) +{ +g_slice_free(DomainSaveToFileData, data); +} + +static void +gvir_domain_save_to_file_helper(GSimpleAsyncResult *res, +GObject *object, +GCancellable *cancellable G_GNUC_UNUSED) +{ +GVirDomain *dom = GVIR_DOMAIN(object); +DomainSaveToFileData *data; +GVirConfigDomain *conf; +GError *err = NULL; + // gchar *xml; + +data = g_simple_async_result_get_op_res_gpointer(res); + // //xml = data-filename; +conf = gvir_domain_get_config(dom, data-flags, err); + +if (!gvir_domain_save_to_file_config(dom, data-filename, conf, data-flags, err)) + g_simple_async_result_take_error(res, err); + } + +/** + * gvir_domain_save_to_file_async: + * @dom: the domain + * @flags: the flags + * @cancellable: cancallation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + * + * Asynchronous variant of #gvir_domain_save_to_file. + */ + +void gvir_domain_save_to_file_async (GVirDomain *dom, + gchar *filename, + GVirConfigDomain *conf, + guint flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ +GSimpleAsyncResult *res; +DomainSaveToFileData *data; +gchar *xml; + +g_return_if_fail(GVIR_IS_DOMAIN(dom)); +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN (conf)); +g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + +xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + +data = g_slice_new0(DomainSaveToFileData); +data-filename = filename; +data-custom_xml = xml; +data-flags = flags; + +res = g_simple_async_result_new(G_OBJECT(dom), +callback, +user_data, +
[libvirt] [glib PATCH] Add gvir_connection_domain_restore() and gvir_connection_domain_restore_async()
--- libvirt-gobject/libvirt-gobject-connection.c | 136 ++ libvirt-gobject/libvirt-gobject-connection.h | 19 libvirt-gobject/libvirt-gobject.sym |3 + 3 files changed, 158 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 3a99034..2302328 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -57,6 +57,7 @@ enum { VIR_CONNECTION_CLOSED, VIR_DOMAIN_ADDED, VIR_DOMAIN_REMOVED, +VIR_DOMAIN_RESTORED, LAST_SIGNAL }; @@ -228,6 +229,15 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) 1, GVIR_TYPE_DOMAIN); +signals[VIR_DOMAIN_RESTORED] = g_signal_new(domain-restored, + G_OBJECT_CLASS_TYPE(object_class), +G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET(GVirConnectionClass, domain_restored), +NULL, NULL, + g_cclosure_marshal_VOID__OBJECT, +G_TYPE_NONE, +0); + g_type_class_add_private(klass, sizeof(GVirConnectionPrivate)); } @@ -1605,3 +1615,129 @@ gvir_connection_get_capabilities_finish(GVirConnection *conn, return g_object_ref(caps); } + +/* + * gvir_connection_domain_restore + */ +gboolean gvir_connection_domain_restore(GVirConnection *conn, +gchar *filename, +GVirConfigDomain *conf, +guint flags, +GError **err) +{ +GVirConnectionPrivate *priv; +gchar *custom_xml; +int ret; + +g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE); +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN (conf), FALSE); +g_return_val_if_fail((err == NULL) || (*err == NULL), FALSE); + +priv = conn-priv; +custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + +g_return_val_if_fail(custom_xml != NULL, FALSE); + +if(flags || custom_xml) { + ret = virDomainRestoreFlags(priv-conn, filename, custom_xml, flags); + g_free (custom_xml); +} +else { + ret = virDomainRestore(priv-conn, filename); + g_free (custom_xml); +} + +if(ret 0) { + gvir_set_error_literal(err, GVIR_CONNECTION_ERROR, + 0, + Unable to restore domain); + + return FALSE; +} + +return TRUE; +} + +typedef struct { +gchar *filename; +gchar *custom_xml; +guint flags; +} DomainRestoreFromFileData; + +static void domain_restore_from_file_data_free(DomainRestoreFromFileData *data) +{ +g_slice_free(DomainRestoreFromFileData, data); +} + +static void +gvir_connection_domain_restore_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ +GVirConnection *conn = GVIR_CONNECTION(object); +DomainRestoreFromFileData *data; +GVirConfigDomain *conf; +GError *err = NULL; + +data = g_simple_async_result_get_op_res_gpointer(res); +conf = gvir_config_domain_new_from_xml(data-custom_xml, err); + +if(!gvir_connection_domain_restore(conn, data-filename, conf, data-flags, err)) + g_simple_async_result_take_error(res, err); +} + +/* + * Async function of gvir_connection_domain_restore + */ +void gvir_connection_domain_restore_async(GVirConnection *conn, + gchar *filename, + GVirConfigDomain *conf, + guint flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ +GSimpleAsyncResult *res; +DomainRestoreFromFileData *data; +gchar *custom_xml; + +g_return_if_fail(GVIR_IS_CONNECTION(conn)); +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN (conf)); +g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + +custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + +data = g_slice_new0(DomainRestoreFromFileData); +data-filename = filename; +data-custom_xml = custom_xml; +data-flags = flags; + +res = g_simple_async_result_new(G_OBJECT(conn), +callback, +user_data, +gvir_connection_domain_restore_async); +g_simple_async_result_set_op_res_gpointer(res, data, +
[libvirt] [glib PATCH] Add gvir_connection_domain_restore() and gvir_connection_domain_restore_async()
--- libvirt-gobject/libvirt-gobject-connection.c | 136 ++ libvirt-gobject/libvirt-gobject-connection.h | 19 libvirt-gobject/libvirt-gobject.sym |3 + 3 files changed, 158 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 3a99034..2302328 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -57,6 +57,7 @@ enum { VIR_CONNECTION_CLOSED, VIR_DOMAIN_ADDED, VIR_DOMAIN_REMOVED, +VIR_DOMAIN_RESTORED, LAST_SIGNAL }; @@ -228,6 +229,15 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) 1, GVIR_TYPE_DOMAIN); +signals[VIR_DOMAIN_RESTORED] = g_signal_new(domain-restored, + G_OBJECT_CLASS_TYPE(object_class), +G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET(GVirConnectionClass, domain_restored), +NULL, NULL, + g_cclosure_marshal_VOID__OBJECT, +G_TYPE_NONE, +0); + g_type_class_add_private(klass, sizeof(GVirConnectionPrivate)); } @@ -1605,3 +1615,129 @@ gvir_connection_get_capabilities_finish(GVirConnection *conn, return g_object_ref(caps); } + +/* + * gvir_connection_domain_restore + */ +gboolean gvir_connection_domain_restore(GVirConnection *conn, +gchar *filename, +GVirConfigDomain *conf, +guint flags, +GError **err) +{ +GVirConnectionPrivate *priv; +gchar *custom_xml; +int ret; + +g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE); +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN (conf), FALSE); +g_return_val_if_fail((err == NULL) || (*err == NULL), FALSE); + +priv = conn-priv; +custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + +g_return_val_if_fail(custom_xml != NULL, FALSE); + +if(flags || custom_xml) { + ret = virDomainRestoreFlags(priv-conn, filename, custom_xml, flags); + g_free (custom_xml); +} +else { + ret = virDomainRestore(priv-conn, filename); + g_free (custom_xml); +} + +if(ret 0) { + gvir_set_error_literal(err, GVIR_CONNECTION_ERROR, + 0, + Unable to restore domain); + + return FALSE; +} + +return TRUE; +} + +typedef struct { +gchar *filename; +gchar *custom_xml; +guint flags; +} DomainRestoreFromFileData; + +static void domain_restore_from_file_data_free(DomainRestoreFromFileData *data) +{ +g_slice_free(DomainRestoreFromFileData, data); +} + +static void +gvir_connection_domain_restore_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ +GVirConnection *conn = GVIR_CONNECTION(object); +DomainRestoreFromFileData *data; +GVirConfigDomain *conf; +GError *err = NULL; + +data = g_simple_async_result_get_op_res_gpointer(res); +conf = gvir_config_domain_new_from_xml(data-custom_xml, err); + +if(!gvir_connection_domain_restore(conn, data-filename, conf, data-flags, err)) + g_simple_async_result_take_error(res, err); +} + +/* + * Async function of gvir_connection_domain_restore + */ +void gvir_connection_domain_restore_async(GVirConnection *conn, + gchar *filename, + GVirConfigDomain *conf, + guint flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ +GSimpleAsyncResult *res; +DomainRestoreFromFileData *data; +gchar *custom_xml; + +g_return_if_fail(GVIR_IS_CONNECTION(conn)); +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN (conf)); +g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + +custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + +data = g_slice_new0(DomainRestoreFromFileData); +data-filename = filename; +data-custom_xml = custom_xml; +data-flags = flags; + +res = g_simple_async_result_new(G_OBJECT(conn), +callback, +user_data, +gvir_connection_domain_restore_async); +g_simple_async_result_set_op_res_gpointer(res, data, +