Re: [libvirt] [PATCH] openvz: Handle domain obj hash map errors

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Hendrik Schwartke
---
 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

2012-07-10 Thread Kevin Wolf
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

2012-07-10 Thread Kevin Wolf
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

2012-07-10 Thread Daniel P. Berrange
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.

2012-07-10 Thread tangchen
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

2012-07-10 Thread tangchen
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

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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

2012-07-10 Thread tangchen
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

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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.

2012-07-10 Thread Osier Yang

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.

2012-07-10 Thread tangchen
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.

2012-07-10 Thread tangchen
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread tangchen
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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread tangchen
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

2012-07-10 Thread Daniel P. Berrange
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

2012-07-10 Thread Daniel P. Berrange
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

2012-07-10 Thread Christophe Fergeau
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

2012-07-10 Thread Peter Krempa

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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Christophe Fergeau
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Osier Yang
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

2012-07-10 Thread Hendrik Schwartke
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Osier Yang

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.

2012-07-10 Thread Claudio Bley
 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

2012-07-10 Thread Osier Yang

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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Raghunatha Reddy


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

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Peter Krempa

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

2012-07-10 Thread Jim Fehlig
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

2012-07-10 Thread 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.

-- 
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

2012-07-10 Thread Hendrik Schwartke
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

2012-07-10 Thread Eric Blake
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.

2012-07-10 Thread Michal Privoznik
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

2012-07-10 Thread Dmitry Guryanov

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

2012-07-10 Thread Guido Günther
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

2012-07-10 Thread Eduardo Habkost
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

2012-07-10 Thread Jiri Denemark
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

2012-07-10 Thread Peter Krempa

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

2012-07-10 Thread Guido Günther
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

2012-07-10 Thread Guido Günther
---
 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

2012-07-10 Thread Guido Günther
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

2012-07-10 Thread Guido Günther
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

2012-07-10 Thread Guido Günther
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

2012-07-10 Thread Guido Günther
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Peter Krempa

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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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)

2012-07-10 Thread Eric Blake
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)

2012-07-10 Thread Peter Krempa

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)

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread Eric Blake
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

2012-07-10 Thread tangchen
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

2012-07-10 Thread Doug Goldstein
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 Thread Gao feng
于 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

2012-07-10 Thread Wen Congyang
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

2012-07-10 Thread Alex Jia

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

2012-07-10 Thread Guannan Ren

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

2012-07-10 Thread Jovanka Gulicoska
---
 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()

2012-07-10 Thread Jovanka Gulicoska
---
 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()

2012-07-10 Thread Jovanka Gulicoska
---
 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, 
+