[libvirt] [PATCH] storage_fs: Create directory with UID if needed

2015-05-27 Thread Martin Kletzander
The code already exists there, it just modified different flags.  I just
noticed this when looking at the code.  This patch is better to view
with bigger context or '-W'.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/storage/storage_backend_fs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index bcbbb3ae252a..5dc712925b27 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1,7 +1,7 @@
 /*
  * storage_backend_fs.c: storage backend for FS and directory handling
  *
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -807,7 +807,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 (needs_create_as_uid || !virFileExists(pool-def-target.path)))
 mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE;
 if (needs_create_as_uid)
-flags |= VIR_DIR_CREATE_AS_UID;
+dir_create_flags |= VIR_DIR_CREATE_AS_UID;

 /* Now create the final dir in the path with the uid/gid/mode
  * requested in the config. If the dir already exists, just set
-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] storage_fs: Create directory with UID if needed

2015-05-27 Thread Ján Tomko
On Wed, May 27, 2015 at 10:10:53AM +0200, Martin Kletzander wrote:
 The code already exists there, it just modified different flags.  I just
 noticed this when looking at the code.  This patch is better to view
 with bigger context or '-W'.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/storage/storage_backend_fs.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index bcbbb3ae252a..5dc712925b27 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -1,7 +1,7 @@
  /*
   * storage_backend_fs.c: storage backend for FS and directory handling
   *
 - * Copyright (C) 2007-2014 Red Hat, Inc.
 + * Copyright (C) 2007-2015 Red Hat, Inc.
   * Copyright (C) 2007-2008 Daniel P. Berrange
   *
   * This library is free software; you can redistribute it and/or

Please configure your editor to not generate noise.

 @@ -807,7 +807,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
  (needs_create_as_uid || !virFileExists(pool-def-target.path)))
  mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE;
  if (needs_create_as_uid)
 -flags |= VIR_DIR_CREATE_AS_UID;
 +dir_create_flags |= VIR_DIR_CREATE_AS_UID;
 
  /* Now create the final dir in the path with the uid/gid/mode
   * requested in the config. If the dir already exists, just set

ACK to this hunk.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Libvirt now using Zanata for translation

2015-05-27 Thread Daniel P. Berrange
On Wed, May 27, 2015 at 08:45:09AM +0800, zhang bo wrote:
 On 2015/3/6 21:20, Daniel P. Berrange wrote:
 
  As of current GIT master, libvirt is fully using Zanata for po file
  translation
  
  After some trouble with the zanata python client, I have now successfully
  refreshed  pushed the current libvirt.pot file, and synchronized all
  the translation po files back down to GIT master.
  
  For ongoing refreshes of the libvirt.pot, it can be rebuild  pushed
  to zanata and .po files resynchonized using
  
   # cd po
   # rm libvirt.pot
   # make libvirt.pot
   # zanata-cli push
   # zanata-cli pull
  
  Where 'zanata-cli' is the *JAVA* client, not the python client. I strongly
  recommend against use of the python client for the libvirt project as it
  doesn't handle the size of the libvirt.pot/.po files well resulting in
  unexplained data loss.  Even the java client has problems pushing the
  .po files to the server due to poorly designed rate limiting in the
  zanata server.
  
  Fortunately this is not something we need do again, now we have imported
  the existing .po file.  The java client has been troublefree wrt to pushing
  the .pot file and pulling .po files, which is all we need for ongoing work
  now.
  
  Currently myself  Daniel Veillard are setup as admins of the libvirt
  project in Zanata  we can add further people as required.
  
  Regards,
  Daniel
 
 
 Would you please add me as admin so that I could help to translate and review 
 (in Chinese)?

The libvirt translations are managed under the Fedora translations teams.
So in fact you just need to request to join the Chinese Fedora team. If
you go to this page:

   https://fedora.zanata.org/language/view/zh-CN

There's a drop down menu 'v ...' which has a 'Request to join team'
option in it.

Once you've joined you'd be able to work on any Fedora related project,
or just libvirt, as you so prefer.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] lxc / docker containers gives option to inherit the namespaces. Example lxc-start has option [ --share-[net|ipc|uts] name|pid ] where --share-net name|pid means Inherit a network

2015-05-27 Thread Michal Privoznik
On 21.05.2015 19:43, ik.nitk wrote:
  This patch tries to add the similar option to libvirt lxc. So to inherit 
 namespace from name
  container c2.
  add this into xml.
  lxc:namespace
  sharenet type='name' value='c2'/
  /lxc:namespace
 
  And to inherit namespace from a pid.
  add this into xml.
  lxc:namespace
  sharenet type='pid' value='10245'/
  /lxc:namespace
 
  And to inherit namespace from a netns.
  add this into xml.
  lxc:namespace
  sharenet type='netns' value='red'/
  /lxc:namespace
 
  Similar options for ipc/uts.
  shareipc/ , shareuts /
 
  The reasong lxc xml namespace is added because this feature is very specific 
 to lxc. Therfore wanted to
  keep it seperated from actual libvirt xml domain.
 
  -imran
 ---
  src/Makefile.am |   5 +-
  src/lxc/lxc_conf.c  |   2 +-
  src/lxc/lxc_conf.h  |  23 +
  src/lxc/lxc_container.c | 191 ++--
  src/lxc/lxc_domain.c| 254 
 +++-
  src/lxc/lxc_domain.h|   1 +
  6 files changed, 463 insertions(+), 13 deletions(-)

Unfortunately, we entered freeze and this patch slipped. Sorry. I'll
review after the release.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv2] qemu: add a check for slot and base when build dimm address

2015-05-27 Thread Luyao Huang
When hot-plug a memory device, we don't check if there
is a memory device have the same address with the memory device
we want hot-pluged. Qemu forbid use/hot-plug 2 memory device
with same slot or the same base(qemu side this elemnt named addr).

Introduce a address check when build memory device qemu command line.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
v2:
 move the check to qemuBuildMemoryDeviceStr() to check the dimm address
 when start/hot-plug a memory device.

 src/qemu/qemu_command.c | 77 -
 1 file changed, 57 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d8ce511..0380a3b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4955,6 +4955,61 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
 }
 
 
+static int
+qemuBuildMemoryDeviceAddr(virBuffer *buf,
+  virDomainDefPtr def,
+  virDomainMemoryDefPtr mem)
+{
+ssize_t i;
+
+if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+return 0;
+
+if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(only 'dimm' addresses are supported for the 
+ pc-dimm device));
+return -1;
+}
+
+if (mem-info.addr.dimm.slot = def-mem.memory_slots) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(memory device slot '%u' exceeds slots count '%u'),
+   mem-info.addr.dimm.slot, def-mem.memory_slots);
+return -1;
+}
+
+for (i = 0; i  def-nmems; i++) {
+ virDomainMemoryDefPtr tmp = def-mems[i];
+
+ if (tmp == mem ||
+ tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM)
+ continue;
+
+ if (mem-info.addr.dimm.slot == tmp-info.addr.dimm.slot) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(memory device slot '%u' already been
+   used by other memory device),
+mem-info.addr.dimm.slot);
+ return -1;
+ }
+
+ if (mem-info.addr.dimm.base != 0  tmp-info.addr.dimm.base != 0 
+ mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(memory device base '0x%llx' already been
+   used by other memory device),
+mem-info.addr.dimm.base);
+ return -1;
+ }
+}
+
+virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot);
+virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base);
+
+return 0;
+}
+
 char *
 qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
  virDomainDefPtr def,
@@ -4976,29 +5031,11 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
 return NULL;
 }
 
-if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM 
-mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(only 'dimm' addresses are supported for the 
- pc-dimm device));
-return NULL;
-}
-
-if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM 
-mem-info.addr.dimm.slot = def-mem.memory_slots) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _(memory device slot '%u' exceeds slots count 
'%u'),
-   mem-info.addr.dimm.slot, def-mem.memory_slots);
-return NULL;
-}
-
 virBufferAsprintf(buf, pc-dimm,node=%d,memdev=mem%s,id=%s,
   mem-targetNode, mem-info.alias, mem-info.alias);
 
-if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
-virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot);
-virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base);
-}
+if (qemuBuildMemoryDeviceAddr(buf, def, mem)  0)
+return NULL;
 
 break;
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] lxc: properly clean up qemu-nbd

2015-05-27 Thread Cédric Bosdonnat
Add the qemu-nbd tasks to the container cgroup to make sure those will
be killed when the container is stopped. In order to reliably get the
qemu-nbd tasks PIDs, we use /sys/devices/virtual/block/DEV/pid as
qemu-nbd is daemonizing itself.
---
 src/libvirt_private.syms |  1 +
 src/lxc/lxc_controller.c | 56 
 src/util/virprocess.c| 45 ++
 src/util/virprocess.h|  2 ++
 4 files changed, 104 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8c50ea2..409bb4f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1973,6 +1973,7 @@ virProcessAbort;
 virProcessExitWithStatus;
 virProcessGetAffinity;
 virProcessGetNamespaces;
+virProcessGetPids;
 virProcessGetStartTime;
 virProcessKill;
 virProcessKillPainfully;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index e144c2d..14d873e 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -107,6 +107,9 @@ struct _virLXCController {
 
 pid_t initpid;
 
+size_t nnbdpids;
+pid_t *nbdpids;
+
 size_t nveths;
 char **veths;
 
@@ -283,6 +286,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl)
 virObjectUnref(ctrl-server);
 virLXCControllerFreeFuse(ctrl);
 
+VIR_FREE(ctrl-nbdpids);
+
 virCgroupFree(ctrl-cgroup);
 
 /* This must always be the last thing to be closed */
@@ -471,6 +476,9 @@ static int 
virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs)
 return -1;
 }
 
+/* The NBD device will be cleaned up while the cgroup will end.
+ * For this we need to remember the qemu-nbd pid and add it to
+ * the cgroup*/
 if (virFileNBDDeviceAssociate(fs-src,
   fs-format,
   fs-readonly,
@@ -503,6 +511,9 @@ static int 
virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk)
 return -1;
 }
 
+/* The NBD device will be cleaned up while the cgroup will end.
+ * For this we need to remember the qemu-nbd pid and add it to
+ * the cgroup*/
 if (virFileNBDDeviceAssociate(src,
   format,
   disk-src-readonly,
@@ -525,6 +536,38 @@ static int 
virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk)
 return 0;
 }
 
+static int virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl,
+ const char *dev)
+{
+char *pidpath = NULL;
+pid_t *pids;
+size_t npids;
+size_t i;
+int ret = -1;
+pid_t pid;
+
+if (!STRPREFIX(dev, /dev/) ||
+virAsprintf(pidpath, /sys/devices/virtual/block/%s/pid, dev + 5)  
0)
+goto cleanup;
+
+if (virPidFileReadPath(pidpath, pid)  0)
+goto cleanup;
+
+if (virProcessGetPids(pid, npids, pids)  0)
+goto cleanup;
+
+for (i = 0; i  npids; i++) {
+if (VIR_APPEND_ELEMENT(ctrl-nbdpids, ctrl-nnbdpids, pids[i])  0)
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(pids);
+VIR_FREE(pidpath);
+return ret;
+}
 
 static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
 {
@@ -570,6 +613,9 @@ static int 
virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
 } else if (fs-fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) {
 if (virLXCControllerSetupNBDDeviceFS(fs)  0)
 goto cleanup;
+
+if (virLXCControllerAppendNBDPids(ctrl, fs-src)  0)
+goto cleanup;
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(fs driver %s is not supported),
@@ -629,6 +675,9 @@ static int 
virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
 }
 if (virLXCControllerSetupNBDDeviceDisk(disk)  0)
 goto cleanup;
+
+if (virLXCControllerAppendNBDPids(ctrl, 
virDomainDiskGetSource(disk))  0)
+goto cleanup;
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(disk driver %s is not supported),
@@ -781,6 +830,7 @@ static int 
virLXCControllerSetupCgroupLimits(virLXCControllerPtr ctrl)
 virBitmapPtr auto_nodeset = NULL;
 int ret = -1;
 virBitmapPtr nodeset = NULL;
+size_t i;
 
 VIR_DEBUG(Setting up cgroup resource limits);
 
@@ -798,6 +848,12 @@ static int 
virLXCControllerSetupCgroupLimits(virLXCControllerPtr ctrl)
 if (virCgroupAddTask(ctrl-cgroup, getpid())  0)
 goto cleanup;
 
+/* Add all qemu-nbd tasks to the cgroup */
+for (i = 0; i  ctrl-nnbdpids; i++) {
+if (virCgroupAddTask(ctrl-cgroup, ctrl-nbdpids[i])  0)
+goto cleanup;
+}
+
 if (virLXCCgroupSetup(ctrl-def, ctrl-cgroup, nodeset)  0)
 goto cleanup;
 
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 7a79970..8b4b32f 100644
--- a/src/util/virprocess.c
+++ 

Re: [libvirt] Libvirt now using Zanata for translation

2015-05-27 Thread zhang bo
On 2015/5/27 17:00, Daniel P. Berrange wrote:

 On Wed, May 27, 2015 at 08:45:09AM +0800, zhang bo wrote:
 On 2015/3/6 21:20, Daniel P. Berrange wrote:

 As of current GIT master, libvirt is fully using Zanata for po file
 translation

 After some trouble with the zanata python client, I have now successfully
 refreshed  pushed the current libvirt.pot file, and synchronized all
 the translation po files back down to GIT master.

 For ongoing refreshes of the libvirt.pot, it can be rebuild  pushed
 to zanata and .po files resynchonized using

  # cd po
  # rm libvirt.pot
  # make libvirt.pot
  # zanata-cli push
  # zanata-cli pull

 Where 'zanata-cli' is the *JAVA* client, not the python client. I strongly
 recommend against use of the python client for the libvirt project as it
 doesn't handle the size of the libvirt.pot/.po files well resulting in
 unexplained data loss.  Even the java client has problems pushing the
 .po files to the server due to poorly designed rate limiting in the
 zanata server.

 Fortunately this is not something we need do again, now we have imported
 the existing .po file.  The java client has been troublefree wrt to pushing
 the .pot file and pulling .po files, which is all we need for ongoing work
 now.

 Currently myself  Daniel Veillard are setup as admins of the libvirt
 project in Zanata  we can add further people as required.

 Regards,
 Daniel


 Would you please add me as admin so that I could help to translate and 
 review (in Chinese)?
 
 The libvirt translations are managed under the Fedora translations teams.
 So in fact you just need to request to join the Chinese Fedora team. If
 you go to this page:
 
https://fedora.zanata.org/language/view/zh-CN
 
 There's a drop down menu 'v ...' which has a 'Request to join team'
 option in it.
 
 Once you've joined you'd be able to work on any Fedora related project,
 or just libvirt, as you so prefer.
 
 Regards,
 Daniel


Thanks! waiting for the coordinator to accept my application.

-- 
Oscar
oscar.zhan...@huawei.com  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] qemu: Deal with panic device on pSeries.

2015-05-27 Thread John Ferlan


On 05/15/2015 05:35 AM, Andrea Bolognani wrote:
 The guest firmware provides the same functionality as the pvpanic
 device, which is not available in QEMU on pSeries: make sure the
 XML reflects this fact by automatically adding a panic/ element
 when not already present.
 
 On the other hand, unlike the pvpanic device, the guest firmware
 can't be configured, so report an error if an address has been
 provided in the XML.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182388
 ---
  src/qemu/qemu_command.c| 25 -
  src/qemu/qemu_domain.c | 14 ++
  .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  1 +
  .../qemuxml2argv-pseries-nvram.xml |  1 +
  .../qemuxml2argv-pseries-panic-address.xml | 32 
 ++
  .../qemuxml2argv-pseries-panic-missing.args|  7 +
  .../qemuxml2argv-pseries-panic-missing.xml | 29 
  .../qemuxml2argv-pseries-panic-no-address.args |  7 +
  .../qemuxml2argv-pseries-panic-no-address.xml  | 30 
  tests/qemuxml2argvtest.c   |  6 
  .../qemuxml2xmlout-pseries-panic-missing.xml   | 30 
  tests/qemuxml2xmltest.c|  2 ++
  12 files changed, 177 insertions(+), 7 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml
  create mode 100644 
 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
 

This should be split into two patches The first one dealing with
just the error/bug and the second dealing with adding a panic device by
default for PPC*

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 5d0a167..138a8b6 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -10737,13 +10737,28 @@ qemuBuildCommandLine(virConnectPtr conn,
  }
  
  if (def-panic) {
 -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
 +if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
 pseries)) {
 +/* For pSeries guests, the firmware provides the same
 + * functionality of the pvpanic device. The address
 + * cannot be configured by the user */
 +if (def-panic-info.type != 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(setting the panic device address is not 
 + supported for pSeries guests));
 +goto error;
 +}

Seems to be something that should documented in formatdomain.html.in -
that is a panic device for PPC64 can not have an address

 +} else {
 +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(your QEMU is too old to support pvpanic));

Since you're moving it - use the opportunity to change the message to:

This QEMU doesn't support the panic device

Whether you go with pvpanic or panic just depends on how technically
detailed you want to get - libvirt refers to it as panic, while QEMU
refers to it as pvpanic...  I'm ambivalent as to which to use since it
points at the same root issue.

 +goto error;
 +}
 +
  if (def-panic-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) 
 {
  virCommandAddArg(cmd, -device);
  virCommandAddArgFormat(cmd, pvpanic,ioport=%d,
 def-panic-info.addr.isa.iobase);
 -} else if (def-panic-info.type ==
 -   VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
 +} else if (def-panic-info.type == 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {

This change makes the line  80 characters - so it should be changed
back to two lines.

  virCommandAddArgList(cmd, -device, pvpanic, NULL);
  } else {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 @@ -10751,10 +10766,6 @@ qemuBuildCommandLine(virConnectPtr conn,
   with ISA address type));
  goto error;
  }
 -} else {
 -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -   _(your QEMU is too old to support pvpanic));
 -goto error;
  }
  }
  

The above seems to be patch 1 (bug fix) while the rest would be patch
2 (feature change). Although I think swapping the order of 

Re: [libvirt] [PATCH] zfs: fix storagepoolxml2xml test

2015-05-27 Thread Roman Bogorodskiy
  Cole Robinson wrote:

 On 05/26/2015 12:30 AM, Roman Bogorodskiy wrote:
  Commit 7c2d65d dropped setting default mode.
  
  Update zfs tests accordingly.
  ---
   tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml | 3 ---
   tests/storagepoolxml2xmlout/pool-zfs.xml   | 3 ---
   2 files changed, 6 deletions(-)
  
  diff --git a/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml 
  b/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml
  index 89dcdbf..8d9be73 100644
  --- a/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml
  +++ b/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml
  @@ -10,8 +10,5 @@
 /source
 target
   path/dev/zvol/testpool/path
  -permissions
  -  mode0755/mode
  -/permissions
 /target
   /pool
  diff --git a/tests/storagepoolxml2xmlout/pool-zfs.xml 
  b/tests/storagepoolxml2xmlout/pool-zfs.xml
  index c9e5df9..1a8de4f 100644
  --- a/tests/storagepoolxml2xmlout/pool-zfs.xml
  +++ b/tests/storagepoolxml2xmlout/pool-zfs.xml
  @@ -9,8 +9,5 @@
 /source
 target
   path/dev/zvol/testpool/path
  -permissions
  -  mode0755/mode
  -/permissions
 /target
   /pool
  
 
 ACK, sorry about that

Pushed, thanks!

Roman Bogorodskiy

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] debug: assure NULLSTR() around all %s args in debug at top of public APIs

2015-05-27 Thread Laine Stump
There are also a couple that were very uninformatively just logging
the value of the pointer rather than the string itself:

* the name arg to virNodeDeviceLookupByName()
* wwnn and wwpn args to virNodeDeviceLookupSCSIHostByWWN()

All char*'s that make sense should now have their contents logged
rather than the pointer, and all %s args should now be inside
NULLSTR().
---
 src/libvirt-domain.c| 36 ++--
 src/libvirt-host.c  |  6 +++---
 src/libvirt-interface.c |  6 +++---
 src/libvirt-network.c   |  6 +++---
 src/libvirt-nodedev.c   | 10 +-
 src/libvirt-nwfilter.c  |  6 +++---
 src/libvirt-secret.c|  4 ++--
 src/libvirt-storage.c   | 16 
 8 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2edba1a..5907f58 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -167,7 +167,7 @@ virDomainPtr
 virDomainCreateXML(virConnectPtr conn, const char *xmlDesc,
unsigned int flags)
 {
-VIR_DEBUG(conn=%p, xmlDesc=%s, flags=%x, conn, xmlDesc, flags);
+VIR_DEBUG(conn=%p, xmlDesc=%s, flags=%x, conn, NULLSTR(xmlDesc), flags);
 
 virResetLastError();
 
@@ -235,7 +235,7 @@ virDomainCreateXMLWithFiles(virConnectPtr conn, const char 
*xmlDesc,
 unsigned int flags)
 {
 VIR_DEBUG(conn=%p, xmlDesc=%s, nfiles=%u, files=%p, flags=%x,
-  conn, xmlDesc, nfiles, files, flags);
+  conn, NULLSTR(xmlDesc), nfiles, files, flags);
 
 virResetLastError();
 
@@ -413,7 +413,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const char 
*uuidstr)
 virDomainPtr
 virDomainLookupByName(virConnectPtr conn, const char *name)
 {
-VIR_DEBUG(conn=%p, name=%s, conn, name);
+VIR_DEBUG(conn=%p, name=%s, conn, NULLSTR(name));
 
 virResetLastError();
 
@@ -955,7 +955,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
 int
 virDomainRestore(virConnectPtr conn, const char *from)
 {
-VIR_DEBUG(conn=%p, from=%s, conn, from);
+VIR_DEBUG(conn=%p, from=%s, conn, NULLSTR(from));
 
 virResetLastError();
 
@@ -1025,7 +1025,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char 
*from, const char *dxml,
   unsigned int flags)
 {
 VIR_DEBUG(conn=%p, from=%s, dxml=%s, flags=%x,
-  conn, from, NULLSTR(dxml), flags);
+  conn, NULLSTR(from), NULLSTR(dxml), flags);
 
 virResetLastError();
 
@@ -1089,7 +1089,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const 
char *file,
  unsigned int flags)
 {
 VIR_DEBUG(conn=%p, file=%s, flags=%x,
-  conn, file, flags);
+  conn, NULLSTR(file), flags);
 
 virResetLastError();
 
@@ -1162,7 +1162,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const 
char *file,
 const char *dxml, unsigned int flags)
 {
 VIR_DEBUG(conn=%p, file=%s, dxml=%s, flags=%x,
-  conn, file, dxml, flags);
+  conn, NULLSTR(file), NULLSTR(dxml), flags);
 
 virResetLastError();
 
@@ -2623,7 +2623,7 @@ virConnectDomainXMLFromNative(virConnectPtr conn,
   unsigned int flags)
 {
 VIR_DEBUG(conn=%p, format=%s, config=%s, flags=%x,
-  conn, nativeFormat, nativeConfig, flags);
+  conn, NULLSTR(nativeFormat), NULLSTR(nativeConfig), flags);
 
 virResetLastError();
 
@@ -2673,7 +2673,7 @@ virConnectDomainXMLToNative(virConnectPtr conn,
 unsigned int flags)
 {
 VIR_DEBUG(conn=%p, format=%s, xml=%s, flags=%x,
-  conn, nativeFormat, domainXml, flags);
+  conn, NULLSTR(nativeFormat), NULLSTR(domainXml), flags);
 
 virResetLastError();
 
@@ -4597,7 +4597,7 @@ virDomainMigrateFinish(virConnectPtr dconn,
 {
 VIR_DEBUG(dconn=%p, dname=%s, cookie=%p, cookielen=%d, uri=%s, 
   flags=%lx, dconn, NULLSTR(dname), cookie, cookielen,
-  uri, flags);
+  NULLSTR(uri), flags);
 
 virResetLastError();
 
@@ -4639,8 +4639,8 @@ virDomainMigratePrepare2(virConnectPtr dconn,
 {
 VIR_DEBUG(dconn=%p, cookie=%p, cookielen=%p, uri_in=%s, uri_out=%p,
   flags=%lx, dname=%s, bandwidth=%lu, dom_xml=%s, dconn,
-  cookie, cookielen, uri_in, uri_out, flags, NULLSTR(dname),
-  bandwidth, dom_xml);
+  cookie, cookielen, NULLSTR(uri_in), uri_out, flags, 
NULLSTR(dname),
+  bandwidth, NULLSTR(dom_xml));
 
 virResetLastError();
 
@@ -4681,7 +4681,7 @@ virDomainMigrateFinish2(virConnectPtr dconn,
 {
 VIR_DEBUG(dconn=%p, dname=%s, cookie=%p, cookielen=%d, uri=%s, 
   flags=%lx, retcode=%d, dconn, NULLSTR(dname), cookie,
-  cookielen, uri, flags, retcode);
+  cookielen, NULLSTR(uri), flags, retcode);
 
 virResetLastError();
 
@@ -4721,7 +4721,7 @@ virDomainMigratePrepareTunnel(virConnectPtr conn,
 {

[libvirt] [PATCH] node_device: more informative error log when device isn't found

2015-05-27 Thread Laine Stump
In a couple of cases, the node device driver (and the test node device
driver which likely copied it) was only logging Node device not
found when it couldn't find the requested device. This patch changes
those cases to log the name (and in the case when it's relevant, the
wwnn and wwpn) as well.
---
 src/node_device/node_device_driver.c | 14 ++
 src/test/test_driver.c   |  8 ++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 34ba1fa..768db7f 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -248,7 +248,9 @@ nodeDeviceLookupByName(virConnectPtr conn, const char *name)
 nodeDeviceUnlock();
 
 if (!obj) {
-virReportError(VIR_ERR_NO_NODE_DEVICE, NULL);
+virReportError(VIR_ERR_NO_NODE_DEVICE,
+   _(no node device with matching name '%s'),
+   name);
 goto cleanup;
 }
 
@@ -597,8 +599,10 @@ nodeDeviceCreateXML(virConnectPtr conn,
  * we're returning what we get... */
 
 if (dev == NULL)
-virReportError(VIR_ERR_NO_NODE_DEVICE, NULL);
-
+virReportError(VIR_ERR_NO_NODE_DEVICE,
+   _(no node device for '%s' with matching 
+ wwnn '%s' and wwpn '%s'),
+   def-name, wwnn, wwpn);
  cleanup:
 nodeDeviceUnlock();
 virNodeDeviceDefFree(def);
@@ -621,7 +625,9 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
 nodeDeviceUnlock();
 
 if (!obj) {
-virReportError(VIR_ERR_NO_NODE_DEVICE, NULL);
+virReportError(VIR_ERR_NO_NODE_DEVICE,
+   _(no node device with matching name '%s'),
+   dev-name);
 goto out;
 }
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 038b2b8..d1f0af3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5673,7 +5673,9 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char 
*name)
 testDriverUnlock(driver);
 
 if (!obj) {
-virReportError(VIR_ERR_NO_NODE_DEVICE, NULL);
+virReportError(VIR_ERR_NO_NODE_DEVICE,
+   _(no node device with matching name '%s'),
+   name);
 goto cleanup;
 }
 
@@ -5893,7 +5895,9 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
 testDriverUnlock(driver);
 
 if (!obj) {
-virReportError(VIR_ERR_NO_NODE_DEVICE, NULL);
+virReportError(VIR_ERR_NO_NODE_DEVICE,
+   _(no node device with matching name '%s'),
+   dev-name);
 goto out;
 }
 
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] check if console PTY is null before attempting to open

2015-05-27 Thread Shivaprasad G Bhat
Console devices have their pty devices assigned when the qemu is actually
started. The libvirt spends considerable amount of time to start the qemu if
the guest has multiple passthrough devices. If time is spent during the
hostdev preparation, someone attempts to open the console, the libvirt
crashes in virChrdevLockFilePath().The patch attempts to fix the crash by
adding a check before attempting to open.

Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
 src/qemu/qemu_driver.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1233d8f..fec928f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16278,6 +16278,12 @@ qemuDomainOpenConsole(virDomainPtr dom,
 goto cleanup;
 }
 
+if (!(chr-source.data.file.path)) {
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+   _(PTY device is not yet assigned));
+goto cleanup;
+}
+
 /* handle mutually exclusive access to console devices */
 ret = virChrdevOpen(priv-devs,
 chr-source,

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH libvirt] interface: don't error out if a bond has no interfaces

2015-05-27 Thread Lubomir Rintel
It's not a problem at all and causes virt-manager to break down.

Note: netcf 0.2.8 generates invalid XML for a bond with no interfaces anyway,
so this error is in fact not reached as we fail earlier. Fix submitted upstream.

Signed-off-by: Lubomir Rintel lkund...@v3.sk
---
 src/conf/interface_conf.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index c2eb945..29769ac 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -553,19 +553,15 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def,
 nbItf = virXPathNodeSet(./interface, ctxt, interfaces);
 if (nbItf  0) {
 ret = -1;
-goto error;
+goto cleanup;
 }
 
-if (nbItf == 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   %s, _(bond has no interfaces));
-ret = -1;
-goto error;
-}
+if (nbItf == 0)
+goto cleanup;
 
 if (VIR_ALLOC_N(def-data.bond.itf, nbItf)  0) {
 ret = -1;
-goto error;
+goto cleanup;
 }
 def-data.bond.nbItf = nbItf;
 
@@ -575,12 +571,12 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def,
 if (itf == NULL) {
 ret = -1;
 def-data.bond.nbItf = i;
-goto error;
+goto cleanup;
 }
 def-data.bond.itf[i] = itf;
 }
 
- error:
+ cleanup:
 VIR_FREE(interfaces);
 ctxt-node = bond;
 return ret;
-- 
2.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH libvirt] interface: don't error out if a bond has no interfaces

2015-05-27 Thread Laine Stump
On 05/27/2015 01:30 PM, Lubomir Rintel wrote:
 It's not a problem at all and causes virt-manager to break down.

 Note: netcf 0.2.8 generates invalid XML for a bond with no interfaces anyway,
 so this error is in fact not reached as we fail earlier. Fix submitted 
 upstream.

ACK.

This patch also makes bonds more consistent with bridges, which also
require the bridge element, but allow it to be empty.

Since this is a bugfix and straightforward, I'm pushing it now (after
adding a small bit to the commit log, and changing ret initialization
from 0 to -1, so that it's more in line with the rest of libvirt's
code), and will be looking at the netcf patch momentarily.

Thanks for taking the time to report this error with a patch :-)


 Signed-off-by: Lubomir Rintel lkund...@v3.sk
 ---
  src/conf/interface_conf.c | 16 ++--
  1 file changed, 6 insertions(+), 10 deletions(-)

 diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
 index c2eb945..29769ac 100644
 --- a/src/conf/interface_conf.c
 +++ b/src/conf/interface_conf.c
 @@ -553,19 +553,15 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def,
  nbItf = virXPathNodeSet(./interface, ctxt, interfaces);
  if (nbItf  0) {
  ret = -1;
 -goto error;
 +goto cleanup;
  }
  
 -if (nbItf == 0) {
 -virReportError(VIR_ERR_XML_ERROR,
 -   %s, _(bond has no interfaces));
 -ret = -1;
 -goto error;
 -}
 +if (nbItf == 0)
 +goto cleanup;
  
  if (VIR_ALLOC_N(def-data.bond.itf, nbItf)  0) {
  ret = -1;
 -goto error;
 +goto cleanup;
  }
  def-data.bond.nbItf = nbItf;
  
 @@ -575,12 +571,12 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def,
  if (itf == NULL) {
  ret = -1;
  def-data.bond.nbItf = i;
 -goto error;
 +goto cleanup;
  }
  def-data.bond.itf[i] = itf;
  }
  
 - error:
 + cleanup:
  VIR_FREE(interfaces);
  ctxt-node = bond;
  return ret;

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Plans for next release

2015-05-27 Thread Jim Fehlig

On 05/23/2015 07:22 AM, Daniel Veillard wrote:

Hi everybody,

if we want to get it by next month, we should probably freeze on Tuesday
for an 1.2.16 on June 1st, we 'only' have 137 commits since 1.2.15 but
sticking to the monthly release is important.



Hi Daniel,

I have a few old patches on the list that would be nice to get into 1.2.16.  
First is the libxl config file fixes in the spec file that we postponed during 
the 1.2.15 release


https://www.redhat.com/archives/libvir-list/2015-May/msg4.html

Next is a patch to fix generation of AUTHORS file in the python bindings

https://www.redhat.com/archives/libvir-list/2015-May/msg00319.html

Finally, since the domXML - xl.cfg conversions for SPICE where committed a few 
weeks back, it would be nice to provide support for SPICE and qxl config in the 
libxl driver too


https://www.redhat.com/archives/libvir-list/2015-May/msg00256.html

It would be unfortunate to convert xl.cfg SPICE config to domXML but then be 
unable to use that config in the libxl driver.


Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize

2015-05-27 Thread John Ferlan


On 05/27/2015 11:08 AM, Ján Tomko wrote:
 The volume cannot be shrinked below existing allocation, thus
 a successful resize with VOL_RESIZE_ALLOCATE will never increase
 the pool's available value.

Since shrinking a volume below existing allocation is not allowed, it is
not possible for a successful resize with VOL_RESIZE_ALLOCATE to
increase the pool's available value.

 
 Even with the SHRINK flag it is possible to extend the current
 allocation or even the capacity. Remove the overflow when
 computing delta with this flag and do the check even if the
 flag was specified.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1073305

Editorial comment:

This bz should go back to POST...

 ---
  src/storage/storage_driver.c | 21 +
  1 file changed, 5 insertions(+), 16 deletions(-)
 
 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
 index ac4a74a..fbb8050 100644
 --- a/src/storage/storage_driver.c
 +++ b/src/storage/storage_driver.c
 @@ -2292,7 +2292,7 @@ storageVolResize(virStorageVolPtr obj,
  virStorageBackendPtr backend;
  virStoragePoolObjPtr pool = NULL;
  virStorageVolDefPtr vol = NULL;
 -unsigned long long abs_capacity, delta;
 +unsigned long long abs_capacity, delta = 0;
  int ret = -1;
  
  virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE |
 @@ -2341,18 +2341,14 @@ storageVolResize(virStorageVolPtr obj,
  goto cleanup;
  }
  
 -if (flags  VIR_STORAGE_VOL_RESIZE_SHRINK)
 -delta = vol-target.allocation - abs_capacity;
 -else
 +if (flags  VIR_STORAGE_VOL_RESIZE_ALLOCATE)
  delta = abs_capacity - vol-target.allocation;
  
  /* If the operation is going to increase the allocation value and not
   * just the capacity value, then let's make sure there's enough space
   * in the pool in order to perform that operation
   */

The comment won't make sense any more as well.

ACK

I would think safe for freeze

John

 -if (flags  VIR_STORAGE_VOL_RESIZE_ALLOCATE 
 -!(flags  VIR_STORAGE_VOL_RESIZE_SHRINK) 
 -delta  pool-def-available) {
 +if (delta  pool-def-available) {
  virReportError(VIR_ERR_OPERATION_FAILED, %s,
 _(Not enough space left in storage pool));
  goto cleanup;
 @@ -2375,15 +2371,8 @@ storageVolResize(virStorageVolPtr obj,
   */
  if (flags  VIR_STORAGE_VOL_RESIZE_ALLOCATE) {
  vol-target.allocation = abs_capacity;
 -
 -/* Update pool metadata */
 -if (flags  VIR_STORAGE_VOL_RESIZE_SHRINK) {
 -   pool-def-allocation -= delta;
 -   pool-def-available += delta;
 -} else {
 -   pool-def-allocation += delta;
 -   pool-def-available -= delta;
 -}
 +pool-def-allocation += delta;
 +pool-def-available -= delta;
  }
  
  ret = 0;
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/3] Fix shrinking volumes with the delta flag

2015-05-27 Thread John Ferlan


On 05/27/2015 11:08 AM, Ján Tomko wrote:
 This never worked.
 
 In 0.9.10 when this API was introduced, it was intended that
 the SHRINK flag combined with DELTA would shrink the volume by
 the specified capacity (to avoid passing negative numbers).
 See commit 055bbf4.
 
 When the SHRINK flag was finally implemented for the first backend
 in 1.2.13 (commit aa9aa6a), it was only implemented for the absolute
 values and with the delta flag the volume is always extended,
 regardless of the SHRINK flag.
 
 Treat the SHRINK flag as a minus sign when used together with DELTA,
 to allow shrinking volumes as was documented in the API since 0.9.10.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1220213
 ---
  src/storage/storage_driver.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 

ACK (seemingly safe too)

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] virsh: make negative values with vol-resize more convenient

2015-05-27 Thread John Ferlan


On 05/27/2015 11:08 AM, Ján Tomko wrote:
 When shrinking a volume by a certain size, instead of typing
   vol-resize volume 1G --delta --shrink
 we allow the convience of specifying a negative value:
   vol-resize volume -1G --delta --shrink
 getting the same results with one more character.
 
 A negative value only makes sense as a delta. Imply the
 --delta parameter if the value is negative.
 
 Still require --shrink, because the operation is potentially
 destructive.
 ---
  tools/virsh-volume.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)
 

Should virsh.pod be updated as well to indicate that --shrink with
negative number implies --delta (not required, but the text in virsh.pod
around this is lacking...


ACK (seemingly safe too)

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] libxl: load on FreeBSD

2015-05-27 Thread Martin Kletzander

On Sun, May 24, 2015 at 06:45:02PM +0300, Roman Bogorodskiy wrote:

The libxl tries to check if it's running in dom0 by parsing
/proc/xen/capabilities and if that fails it doesn't load.

There's no procfs interface in Xen on FreeBSD, so this check always
fails.

In addition to checking procfs, check if /dev/xen/xenstored, that's enough to
check if we're running in dom0 in FreeBSD case.
---
src/libxl/libxl_driver.c | 42 ++
1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 12be816..fddafa1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -74,6 +74,7 @@ VIR_LOG_INIT(libxl.libxl_driver);
#define LIBXL_CONFIG_FORMAT_SEXPR xen-sxpr

#define HYPERVISOR_CAPABILITIES /proc/xen/capabilities
+#define HYPERVISOR_XENSTORED /dev/xen/xenstored

/* Number of Xen scheduler parameters */
#define XEN_SCHED_CREDIT_NPARAM   2
@@ -427,8 +428,6 @@ static bool
libxlDriverShouldLoad(bool privileged)
{
bool ret = false;
-int status;
-char *output = NULL;

/* Don't load if non-root */
if (!privileged) {
@@ -436,24 +435,27 @@ libxlDriverShouldLoad(bool privileged)
return ret;
}

-if (!virFileExists(HYPERVISOR_CAPABILITIES)) {
-VIR_INFO(Disabling driver as  HYPERVISOR_CAPABILITIES
-  does not exist);
-return ret;
-}
-/*
- * Don't load if not running on a Xen control domain (dom0). It is not
- * sufficient to check for the file to exist as any guest can mount
- * xenfs to /proc/xen.
- */
-status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output);
-if (status = 0)
-status = strncmp(output, control_d, 9);
-VIR_FREE(output);
-if (status) {
-VIR_INFO(No Xen capabilities detected, probably not running 
- in a Xen Dom0.  Disabling libxenlight driver);
-
+if (virFileExists(HYPERVISOR_CAPABILITIES)) {
+int status;
+char *output = NULL;
+/*
+ * Don't load if not running on a Xen control domain (dom0). It is not
+ * sufficient to check for the file to exist as any guest can mount
+ * xenfs to /proc/xen.
+ */
+status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output);
+if (status = 0)
+status = strncmp(output, control_d, 9);
+VIR_FREE(output);
+if (status) {
+VIR_INFO(No Xen capabilities detected, probably not running 
+ in a Xen Dom0.  Disabling libxenlight driver);
+
+return ret;
+}
+} else if (!virFileExists(HYPERVISOR_XENSTORED)) {
+VIR_INFO(Disabling driver as neither  HYPERVISOR_CAPABILITIES
+  nor  HYPERVISOR_CAPABILITIES  exist);


s/HYPERVISOR_CAPABILITIES/HYPERVISOR_XENSTORED/

ACK with that changed.


return ret;
}

--
2.3.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/3] virsh: Pass vshControl to all vshCommandOpt*() calls.

2015-05-27 Thread Andrea Bolognani
On Wed, 2015-05-27 at 16:47 +0200, Michal Privoznik wrote:

  -bool config = vshCommandOptBool(cmd, config);
  +bool config = vshCommandOptBool(ctl, cmd, config);
 
 I don't think this is needed. vshCommandOptBool should never return an
 error. Well, it's returning just if a flag was specified or not.

I added it in to keep all vshCommandOpt*() calls consistent, but since
both you and Peter feel like it shouldn't be there I'll get rid of it.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team
$ python -c print('a'.join(['', 'bologn', '@redh', 't.com']))

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] storage_fs: Create directory with UID if needed

2015-05-27 Thread Cole Robinson
On 05/27/2015 04:10 AM, Martin Kletzander wrote:
 The code already exists there, it just modified different flags.  I just
 noticed this when looking at the code.  This patch is better to view
 with bigger context or '-W'.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/storage/storage_backend_fs.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index bcbbb3ae252a..5dc712925b27 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -1,7 +1,7 @@
  /*
   * storage_backend_fs.c: storage backend for FS and directory handling
   *
 - * Copyright (C) 2007-2014 Red Hat, Inc.
 + * Copyright (C) 2007-2015 Red Hat, Inc.
   * Copyright (C) 2007-2008 Daniel P. Berrange
   *
   * This library is free software; you can redistribute it and/or
 @@ -807,7 +807,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
  (needs_create_as_uid || !virFileExists(pool-def-target.path)))
  mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE;
  if (needs_create_as_uid)
 -flags |= VIR_DIR_CREATE_AS_UID;
 +dir_create_flags |= VIR_DIR_CREATE_AS_UID;
 
  /* Now create the final dir in the path with the uid/gid/mode
   * requested in the config. If the dir already exists, just set
 

ACK, thanks for catching that

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv3] util: make it more robust to calculate timeout value

2015-05-27 Thread Martin Kletzander

On Mon, May 25, 2015 at 02:22:42PM +0800, Wang Yufei wrote:

From: Zhang Bo oscar.zhan...@huawei.com

When we change system clock to years ago, a certain CPU may use up 100% cputime.
The reason is that in function virEventPollCalculateTimeout(), we assign the
unsigned long long result to an INT variable,
   *timeout = then - now; // timeout is INT, and then/now are long long
   if (*timeout  0)
   *timeout = 0;
there's a chance that variable @then minus variable @now may be a very large 
number
that overflows INT value expression, then *timeout will be negative and be 
assigned to 0.
Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' 
while loop there.
thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.

Although as we discussed before in 
https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
it should be prohibited to set-time while other applications are running, but 
it does
seems to have no harm to make the codes more robust.

Signed-off-by: Wang Yufei james.wangyu...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/vireventpoll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index ffda206..4e4f407 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
return -1;

EVENT_DEBUG(Schedule timeout then=%llu now=%llu, then, now);
-*timeout = then - now;
-if (*timeout  0)
+if (then = now)
*timeout = 0;
+else
+*timeout = ((then-now)  INT_MAX) ? INT_MAX : (then-now);


This will work, although the following would be nicer to read since
there were such problems with the calculation in earlier versions:

 if (then = now) {
 *timeout = 0;
 } else {
 long long tmp = then - now;
 if (tmp  INT_MAX)
 *timeout = INT_MAX
 else
 *timeout = tmp;
 }

But that, of course, has the same meaning as your version, which is
fine.  ACK to that, I'll push it in a while.


} else {
*timeout = -1;
}
--
1.7.12.4


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 3/3] virsh: Move error messages inside vshCommandOpt*() functions.

2015-05-27 Thread Michal Privoznik
On 22.05.2015 10:59, Andrea Bolognani wrote:
 ---
  tests/vcpupin|   4 +-
  tools/virsh-domain-monitor.c |   9 +--
  tools/virsh-domain.c | 134 
 +++
  tools/virsh-host.c   |  57 +++---
  tools/virsh-interface.c  |   6 +-
  tools/virsh-network.c|   6 +-
  tools/virsh-volume.c |  24 ++--
  tools/virsh.c| 111 +--
  8 files changed, 104 insertions(+), 247 deletions(-)

Nice cleanup.

 diff --git a/tools/virsh.c b/tools/virsh.c
 index 9f44793..db9354c 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -1517,23 +1517,27 @@ vshCommandOpt(const vshCmd *cmd,
   * 0 in all other cases (@value untouched)
   */

Does it makes sense to note in comments that this function (and others that 
you're fixing) is reporting an error?

  int
 -vshCommandOptInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd,
 +vshCommandOptInt(vshControl *ctl, const vshCmd *cmd,
   const char *name, int *value)
  {
  vshCmdOpt *arg;
  int ret;
  
 -ret = vshCommandOpt(cmd, name, arg, true);
 -if (ret = 0)
 +if ((ret = vshCommandOpt(cmd, name, arg, true)) = 0)
  return ret;
  
 -if (virStrToLong_i(arg-data, NULL, 10, value)  0)
 -return -1;
 -return 1;
 +if ((ret = virStrToLong_i(arg-data, NULL, 10, value))  0)
 +vshError(ctl,
 + _(Numeric value '%s' for %s option is malformed or out 
 of range),
 + arg-data, name);
 +else
 +ret = 1;
 +
 +return ret;
  }
  

While reworking this, you've missed vshCommandOptTimeoutToMs() which in case of 
something like following '--timeout blah' will report error twice: from both 
OptInt() and OptTimeoutToMs():

error: Numeric value 'blah' for timeout option is malformed or out of range
error: invalid timeout

I think this should be squashed in:

@@ -1906,11 +1907,13 @@ vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd 
*cmd,
 {
 int rv = vshCommandOptInt(ctl, cmd, timeout, timeout);
 
-if (rv  0 || (rv  0  *timeout  1)) {
-vshError(ctl, %s, _(invalid timeout));
+if (rv  0)
 return -1;
-}
 if (rv  0) {
+if (*timeout  1) {
+vshError(ctl, %s, _(invalid timeout));
+return -1;
+}
 /* Ensure that we can multiply by 1000 without overflowing. */
 if (*timeout  INT_MAX / 1000) {
 vshError(ctl, %s, _(timeout is too big));

  static int
 -vshCommandOptULongLongInternal(vshControl *ctl ATTRIBUTE_UNUSED, const 
 vshCmd *cmd,
 +vshCommandOptULongLongInternal(vshControl *ctl, const vshCmd *cmd,
 const char *name, unsigned long long *value,
 bool wrap)
  {
 @@ -1754,15 +1767,18 @@ vshCommandOptULongLongInternal(vshControl *ctl 
 ATTRIBUTE_UNUSED, const vshCmd *c
  if ((ret = vshCommandOpt(cmd, name, arg, true)) = 0)
  return ret;
  
 -if (wrap) {
 -if (virStrToLong_ull(arg-data, NULL, 10, value)  0)
 -return -1;
 -} else {
 -if (virStrToLong_ullp(arg-data, NULL, 10, value)  0)
 -return -1;
 -}
 +if (wrap)
 +ret = virStrToLong_ull(arg-data, NULL, 10, value);
 +else
 +ret = virStrToLong_ullp(arg-data, NULL, 10, value);
 +if (ret  0)
 +vshError(ctl,
 + _(Numeric value '%s' for %s option is malformed or out 
 of range),
 + arg-data, name);
 +else
 +ret = 1;
  
 -return 1;
 +return ret;
  }

You've missed one ocurrance of vshCommandOptULongLong in cmdAllocpages().

  
  /**
 @@ -1812,7 +1828,7 @@ vshCommandOptULongLongWrap(vshControl *ctl, const 
 vshCmd *cmd,
   * See vshCommandOptInt()
   */
  int
 -vshCommandOptScaledInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd,
 +vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd,
 const char *name, unsigned long long *value,
 int scale, unsigned long long max)
  {
 @@ -1825,9 +1841,16 @@ vshCommandOptScaledInt(vshControl *ctl 
 ATTRIBUTE_UNUSED, const vshCmd *cmd,
  
  if (virStrToLong_ull(arg-data, end, 10, value)  0 ||
  virScaleInteger(value, end, scale, max)  0)
 -return -1;
 +{
 +vshError(ctl,
 + _(Numeric value '%s' for %s option is malformed or out 
 of range),
 + arg-data, name);
 +ret = -1;
 +} else {
 +ret = 1;
 +}
  
 -return 1;
 +return ret;
  }
  
  
 

Interestingly vshCommandOptString() is missing. So far, the only way for the 
function to fail is if one uses --option  and VSH_OFLAG_EMPTY_OK is not 
specified. So if we come up with good error message for that case, we are okay 
to drop all the other error messages.

Otherwise looking good.

Michal

--
libvir-list mailing list
libvir-list@redhat.com

[libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize

2015-05-27 Thread Ján Tomko
The volume cannot be shrinked below existing allocation, thus
a successful resize with VOL_RESIZE_ALLOCATE will never increase
the pool's available value.

Even with the SHRINK flag it is possible to extend the current
allocation or even the capacity. Remove the overflow when
computing delta with this flag and do the check even if the
flag was specified.

https://bugzilla.redhat.com/show_bug.cgi?id=1073305
---
 src/storage/storage_driver.c | 21 +
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ac4a74a..fbb8050 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2292,7 +2292,7 @@ storageVolResize(virStorageVolPtr obj,
 virStorageBackendPtr backend;
 virStoragePoolObjPtr pool = NULL;
 virStorageVolDefPtr vol = NULL;
-unsigned long long abs_capacity, delta;
+unsigned long long abs_capacity, delta = 0;
 int ret = -1;
 
 virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE |
@@ -2341,18 +2341,14 @@ storageVolResize(virStorageVolPtr obj,
 goto cleanup;
 }
 
-if (flags  VIR_STORAGE_VOL_RESIZE_SHRINK)
-delta = vol-target.allocation - abs_capacity;
-else
+if (flags  VIR_STORAGE_VOL_RESIZE_ALLOCATE)
 delta = abs_capacity - vol-target.allocation;
 
 /* If the operation is going to increase the allocation value and not
  * just the capacity value, then let's make sure there's enough space
  * in the pool in order to perform that operation
  */
-if (flags  VIR_STORAGE_VOL_RESIZE_ALLOCATE 
-!(flags  VIR_STORAGE_VOL_RESIZE_SHRINK) 
-delta  pool-def-available) {
+if (delta  pool-def-available) {
 virReportError(VIR_ERR_OPERATION_FAILED, %s,
_(Not enough space left in storage pool));
 goto cleanup;
@@ -2375,15 +2371,8 @@ storageVolResize(virStorageVolPtr obj,
  */
 if (flags  VIR_STORAGE_VOL_RESIZE_ALLOCATE) {
 vol-target.allocation = abs_capacity;
-
-/* Update pool metadata */
-if (flags  VIR_STORAGE_VOL_RESIZE_SHRINK) {
-   pool-def-allocation -= delta;
-   pool-def-available += delta;
-} else {
-   pool-def-allocation += delta;
-   pool-def-available -= delta;
-}
+pool-def-allocation += delta;
+pool-def-available -= delta;
 }
 
 ret = 0;
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 0/3] volume resize fixes

2015-05-27 Thread Ján Tomko
First two patches fix bugs and are applicable for the freeze.

Ján Tomko (3):
  Simplify allocation check in storageVolResize
  Fix shrinking volumes with the delta flag
  virsh: make negative values with vol-resize more convenient

 src/storage/storage_driver.c | 26 +-
 tools/virsh-volume.c | 15 ---
 2 files changed, 17 insertions(+), 24 deletions(-)

-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/3] virsh: make negative values with vol-resize more convenient

2015-05-27 Thread Ján Tomko
When shrinking a volume by a certain size, instead of typing
  vol-resize volume 1G --delta --shrink
we allow the convience of specifying a negative value:
  vol-resize volume -1G --delta --shrink
getting the same results with one more character.

A negative value only makes sense as a delta. Imply the
--delta parameter if the value is negative.

Still require --shrink, because the operation is potentially
destructive.
---
 tools/virsh-volume.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 5d6cc6a..989ce87 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -1124,14 +1124,10 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
 unsigned long long capacity = 0;
 unsigned int flags = 0;
 bool ret = false;
-bool delta = false;
+bool delta = vshCommandOptBool(cmd, delta);
 
 if (vshCommandOptBool(cmd, allocate))
 flags |= VIR_STORAGE_VOL_RESIZE_ALLOCATE;
-if (vshCommandOptBool(cmd, delta)) {
-delta = true;
-flags |= VIR_STORAGE_VOL_RESIZE_DELTA;
-}
 if (vshCommandOptBool(cmd, shrink))
 flags |= VIR_STORAGE_VOL_RESIZE_SHRINK;
 
@@ -1144,14 +1140,19 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
 if (*capacityStr == '-') {
 /* The API always requires a positive value; but we allow a
  * negative value for convenience.  */
-if (delta  vshCommandOptBool(cmd, shrink)) {
+if (vshCommandOptBool(cmd, shrink)) {
 capacityStr++;
+delta = true;
 } else {
 vshError(ctl, %s,
- _(negative size requires --delta and --shrink));
+ _(negative size requires --shrink));
 goto cleanup;
 }
 }
+
+if (delta)
+flags |= VIR_STORAGE_VOL_RESIZE_DELTA;
+
 if (vshVolSize(capacityStr, capacity)  0) {
 vshError(ctl, _(Malformed size %s), capacityStr);
 goto cleanup;
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/3] Fix shrinking volumes with the delta flag

2015-05-27 Thread Ján Tomko
This never worked.

In 0.9.10 when this API was introduced, it was intended that
the SHRINK flag combined with DELTA would shrink the volume by
the specified capacity (to avoid passing negative numbers).
See commit 055bbf4.

When the SHRINK flag was finally implemented for the first backend
in 1.2.13 (commit aa9aa6a), it was only implemented for the absolute
values and with the delta flag the volume is always extended,
regardless of the SHRINK flag.

Treat the SHRINK flag as a minus sign when used together with DELTA,
to allow shrinking volumes as was documented in the API since 0.9.10.

https://bugzilla.redhat.com/show_bug.cgi?id=1220213
---
 src/storage/storage_driver.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index fbb8050..1ba6828 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2320,7 +2320,10 @@ storageVolResize(virStorageVolPtr obj,
 }
 
 if (flags  VIR_STORAGE_VOL_RESIZE_DELTA) {
-abs_capacity = vol-target.capacity + capacity;
+if (flags  VIR_STORAGE_VOL_RESIZE_SHRINK)
+abs_capacity = vol-target.capacity - MIN(capacity, 
vol-target.capacity);
+else
+abs_capacity = vol-target.capacity + capacity;
 flags = ~VIR_STORAGE_VOL_RESIZE_DELTA;
 } else {
 abs_capacity = capacity;
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] qemu: Limit rtc-reset-reinjection requirement to x86 only.

2015-05-27 Thread Martin Kletzander

On Tue, May 26, 2015 at 06:13:40PM +0200, Andrea Bolognani wrote:

The QMP command, like the interrupt reinjection logic it's connected
to, is only implemented in QEMU when TARGET_I386 is defined, so
checking for its availability on any other architecture is pointless.

On the other hand, when we're on x86, we shouldn still make sure that
rtc-reset-reinjection is available and refuse to set the time
otherwise.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1211938
---
src/qemu/qemu_driver.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)



ACK, will push in a while.


diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..db72bad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18945,7 +18945,12 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}

-if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
+/* On x86, the rtc-reset-reinjection QMP command must be called after
+ * setting the time to avoid trouble down the line. If the command is
+ * not available, don't set the time at all and report an error */
+if (ARCH_IS_X86(vm-def-os.arch) 
+!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION))
+{
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
   _(cannot set time: qemu doesn't support 
 rtc-reset-reinjection command));
@@ -18968,13 +18973,16 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}

-qemuDomainObjEnterMonitor(driver, vm);
-rv = qemuMonitorRTCResetReinjection(priv-mon);
-if (qemuDomainObjExitMonitor(driver, vm)  0)
-goto endjob;
+/* Don't try to call rtc-reset-reinjection if it's not available */
+if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv-mon);
+if (qemuDomainObjExitMonitor(driver, vm)  0)
+goto endjob;

-if (rv  0)
-goto endjob;
+if (rv  0)
+goto endjob;
+}

ret = 0;

--
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3] util: make it more robust to calculate timeout value

2015-05-27 Thread Martin Kletzander

On Wed, May 27, 2015 at 04:58:22PM +0200, Martin Kletzander wrote:

On Mon, May 25, 2015 at 02:22:42PM +0800, Wang Yufei wrote:

From: Zhang Bo oscar.zhan...@huawei.com

When we change system clock to years ago, a certain CPU may use up 100% cputime.
The reason is that in function virEventPollCalculateTimeout(), we assign the
unsigned long long result to an INT variable,
  *timeout = then - now; // timeout is INT, and then/now are long long
  if (*timeout  0)
  *timeout = 0;
there's a chance that variable @then minus variable @now may be a very large 
number
that overflows INT value expression, then *timeout will be negative and be 
assigned to 0.
Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' 
while loop there.
thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.

Although as we discussed before in 
https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
it should be prohibited to set-time while other applications are running, but 
it does
seems to have no harm to make the codes more robust.

Signed-off-by: Wang Yufei james.wangyu...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/vireventpoll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index ffda206..4e4f407 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
   return -1;

   EVENT_DEBUG(Schedule timeout then=%llu now=%llu, then, now);
-*timeout = then - now;
-if (*timeout  0)
+if (then = now)
   *timeout = 0;
+else
+*timeout = ((then-now)  INT_MAX) ? INT_MAX : (then-now);




I added spaces around the minus sign.


This will work, although the following would be nicer to read since
there were such problems with the calculation in earlier versions:

if (then = now) {
*timeout = 0;
} else {
long long tmp = then - now;


And of course this would be unsigned.


if (tmp  INT_MAX)
*timeout = INT_MAX
else
*timeout = tmp;
}

But that, of course, has the same meaning as your version, which is
fine.  ACK to that, I'll push it in a while.


   } else {
   *timeout = -1;
   }
--
1.7.12.4


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


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] maint: document use of zanata for translations

2015-05-27 Thread Eric Blake
Based on recent list questions on how to contribute a translation fix.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Should be safe for freeze, but as I have never contributed a
translation fix, I'll wait for review.

 HACKING  | 19 ---
 docs/hacking.html.in |  7 +++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/HACKING b/HACKING
index fbe838b..e308568 100644
--- a/HACKING
+++ b/HACKING
@@ -18,7 +18,12 @@ listen to feedback.
 and is browsable along with other libvirt-related repositories (e.g.
 libvirt-python) online http://libvirt.org/git/.

-(3) Post patches in unified diff format, with git rename detection enabled. You
+(3) Patches to translations are maintained via the zanata project
+https://fedora.zanata.org/. If you want to fix a translation in a .po file,
+join the appropriate language team. The libvirt release process automatically
+pulls the latest version of each translation file from zanata.
+
+(4) Post patches in unified diff format, with git rename detection enabled. You
 need a one-time setup of:

   git config diff.renames true
@@ -70,7 +75,7 @@ the correct version if needed though).



-(4) In your commit message, make the summary line reasonably short (60 
characters
+(5) In your commit message, make the summary line reasonably short (60 
characters
 is typical), followed by a blank line, followed by any longer description of
 why your patch makes sense. If the patch fixes a regression, and you know what
 commit introduced the problem, mentioning that is useful. If the patch
@@ -82,7 +87,7 @@ is up to you if you want to include or omit them in the 
commit message.



-(5) Split large changes into a series of smaller patches, self-contained if
+(6) 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. Moreover, please keep in mind that it's
 required to be able to compile cleanly (*including* make check and make
@@ -93,10 +98,10 @@ things).



-(6) Make sure your patches apply against libvirt GIT. Developers only follow 
GIT
+(7) Make sure your patches apply against libvirt GIT. Developers only follow 
GIT
 and don't care much about released versions.

-(7) Run the automated tests on your code before submitting any changes. In
+(8) Run the automated tests on your code before submitting any changes. In
 particular, configure with compile warnings set to -Werror. This is done
 automatically for a git checkout; from a tarball, use:

@@ -149,7 +154,7 @@ various tests under gdb or Valgrind.



-(8) The Valgrind test should produce similar output to make check. If the 
output
+(9) The Valgrind test should produce similar output to make check. If the 
output
 has traces within libvirt API's, then investigation is required in order to
 determine the cause of the issue. Output such as the following indicates some
 sort of leak:
@@ -225,7 +230,7 @@ to tests/.valgrind.supp in order to suppress the warning:



-(9) Update tests and/or documentation, particularly if you are adding a new
+(10) Update tests and/or documentation, particularly if you are adding a new
 feature or changing the output of a program.


diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 408ea50..5cd23a2 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -16,6 +16,13 @@
 along with other libvirt-related repositories
 (e.g. libvirt-python) a 
href=http://libvirt.org/git/;online/a./li

+  liPatches to translations are maintained via
+the a href=https://fedora.zanata.org/;zanata project/a.
+If you want to fix a translation in a .po file, join the
+appropriate language team. The libvirt release process
+automatically pulls the latest version of each translation
+file from zanata./li
+
   lipPost patches in unified diff format, with git rename
 detection enabled.  You need a one-time setup of:/p
 pre
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 1/3] virsh: Make vshCommandOptScaledInt() use vshCommandOpt().

2015-05-27 Thread Michal Privoznik
On 22.05.2015 10:59, Andrea Bolognani wrote:
 This aligns it to the other vshCommandOpt*() functions.
 ---
  tools/virsh.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)
 
 diff --git a/tools/virsh.c b/tools/virsh.c
 index 4425774..11c2c30 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -1804,16 +1804,17 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char 
 *name,
 unsigned long long *value, int scale,
 unsigned long long max)
  {
 -const char *str;
 -int ret;
 +vshCmdOpt *arg;
  char *end;
 +int ret;
  
 -ret = vshCommandOptString(cmd, name, str);
 -if (ret = 0)
 +if ((ret = vshCommandOpt(cmd, name, arg, true)) = 0)

This cancels check of arg-def-flags  VSH_OFLAG_EMPTY_OK; but since
this flag makes sense only for string arguments, it's okay.

  return ret;
 -if (virStrToLong_ull(str, end, 10, value)  0 ||
 +
 +if (virStrToLong_ull(arg-data, end, 10, value)  0 ||
  virScaleInteger(value, end, scale, max)  0)
  return -1;
 +
  return 1;
  }
  
 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 2/3] virsh: Pass vshControl to all vshCommandOpt*() calls.

2015-05-27 Thread Michal Privoznik
On 22.05.2015 10:59, Andrea Bolognani wrote:
 This will allow us to use vshError() to report errors from inside
 vshCommandOpt*(), instead of replicating the same logic and error
 messages all over the place.
 
 We also have more context inside the vshCommandOpt*() functions,
 for example the actual value used on the command line, which means
 we can produce more detailed error messages.
 ---
  tools/virsh-domain-monitor.c |  90 +++
  tools/virsh-domain.c | 598 
 +--
  tools/virsh-host.c   |  46 ++--
  tools/virsh-interface.c  |  14 +-
  tools/virsh-network.c|  34 +--
  tools/virsh-nodedev.c|   6 +-
  tools/virsh-pool.c   |  26 +-
  tools/virsh-secret.c |   8 +-
  tools/virsh-snapshot.c   |  88 +++
  tools/virsh-volume.c |  34 +--
  tools/virsh.c| 107 
  tools/virsh.h|  77 +++---
  12 files changed, 574 insertions(+), 554 deletions(-)
 
 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
 index a42c150..db7ef8b 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -317,9 +317,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
  bool ret = false;
  int rv = 0;
  int period = -1;
 -bool config = vshCommandOptBool(cmd, config);
 -bool live = vshCommandOptBool(cmd, live);
 -bool current = vshCommandOptBool(cmd, current);
 +bool config = vshCommandOptBool(ctl, cmd, config);

I don't think this is needed. vshCommandOptBool should never return an
error. Well, it's returning just if a flag was specified or not.

 +bool live = vshCommandOptBool(ctl, cmd, live);
 +bool current = vshCommandOptBool(ctl, cmd, current);
  unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
  
  VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
 @@ -340,7 +340,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
  /* Providing a period will adjust the balloon driver collection period.
   * This is not really an unsigned long, but it
   */
 -if ((rv = vshCommandOptInt(cmd, period, period))  0) {
 +if ((rv = vshCommandOptInt(ctl, cmd, period, period))  0) {

This is of course different, specified value may not be a number in
which case we want an error to be reported.

  vshError(ctl,
   _(Numeric value for %s option is malformed or out of 
 range),
   period);

ACK modulo the OptBool() change.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH V2] libxl: support domainReset

2015-05-27 Thread Jim Fehlig
Currently, libxl does not provide a reset function, but domainReset
can be implemented in the libxl driver by forcibly destroying the
domain and starting it again.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---

This is essentially a V2 of a patch submitted quite some time ago

https://www.redhat.com/archives/libvir-list/2014-August/msg00077.html

The idea of implmenting domainReset in the libxl driver by forcibly
destroying the domain and starting it again was ACK'ed in principle
by Ian Campbell

https://www.redhat.com/archives/libvir-list/2014-August/msg00109.html

I never pushed the patch since it was not ACK'ed by a libvirt
maintainer and stumbled across it while cleaning up some of my
old branches.  The only change here is rebase against current
libvirt.git master.

 src/libxl/libxl_driver.c | 59 
 1 file changed, 59 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 12be816..671d336 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1237,6 +1237,64 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
 }
 
 static int
+libxlDomainReset(virDomainPtr dom, unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom-conn-privateData;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainObjPtr vm;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainResetEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(Domain is not running));
+goto endjob;
+}
+
+/*
+ * The semantics of reset can be achieved by forcibly destroying
+ * the domain and starting it again.
+ */
+if (libxl_domain_destroy(cfg-ctx, vm-def-id, NULL)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Failed to destroy domain '%d' before reset),
+   vm-def-id);
+goto endjob;
+}
+
+libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
+
+if (libxlDomainStart(driver, vm, false, -1)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Failed to start domain '%d' after reset),
+   vm-def-id);
+goto endjob;
+}
+
+ret = 0;
+
+ endjob:
+if (!libxlDomainObjEndJob(driver, vm))
+vm = NULL;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+virObjectUnref(cfg);
+return ret;
+}
+
+static int
 libxlDomainDestroyFlags(virDomainPtr dom,
 unsigned int flags)
 {
@@ -5066,6 +5124,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
 .domainShutdown = libxlDomainShutdown, /* 0.9.0 */
 .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */
 .domainReboot = libxlDomainReboot, /* 0.9.0 */
+.domainReset = libxlDomainReset, /* 1.2.8 */
 .domainDestroy = libxlDomainDestroy, /* 0.9.0 */
 .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */
 .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */
-- 
2.3.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] libxl: load on FreeBSD

2015-05-27 Thread Jim Fehlig

On 05/27/2015 09:06 AM, Martin Kletzander wrote:

On Sun, May 24, 2015 at 06:45:02PM +0300, Roman Bogorodskiy wrote:

The libxl tries to check if it's running in dom0 by parsing
/proc/xen/capabilities and if that fails it doesn't load.

There's no procfs interface in Xen on FreeBSD, so this check always
fails.

In addition to checking procfs, check if /dev/xen/xenstored, that's enough to
check if we're running in dom0 in FreeBSD case.
---
src/libxl/libxl_driver.c | 42 ++
1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 12be816..fddafa1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -74,6 +74,7 @@ VIR_LOG_INIT(libxl.libxl_driver);
#define LIBXL_CONFIG_FORMAT_SEXPR xen-sxpr

#define HYPERVISOR_CAPABILITIES /proc/xen/capabilities
+#define HYPERVISOR_XENSTORED /dev/xen/xenstored

/* Number of Xen scheduler parameters */
#define XEN_SCHED_CREDIT_NPARAM   2
@@ -427,8 +428,6 @@ static bool
libxlDriverShouldLoad(bool privileged)
{
bool ret = false;
-int status;
-char *output = NULL;

/* Don't load if non-root */
if (!privileged) {
@@ -436,24 +435,27 @@ libxlDriverShouldLoad(bool privileged)
return ret;
}

-if (!virFileExists(HYPERVISOR_CAPABILITIES)) {
-VIR_INFO(Disabling driver as  HYPERVISOR_CAPABILITIES
-  does not exist);
-return ret;
-}
-/*
- * Don't load if not running on a Xen control domain (dom0). It is not
- * sufficient to check for the file to exist as any guest can mount
- * xenfs to /proc/xen.
- */
-status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output);
-if (status = 0)
-status = strncmp(output, control_d, 9);
-VIR_FREE(output);
-if (status) {
-VIR_INFO(No Xen capabilities detected, probably not running 
- in a Xen Dom0.  Disabling libxenlight driver);
-
+if (virFileExists(HYPERVISOR_CAPABILITIES)) {
+int status;
+char *output = NULL;
+/*
+ * Don't load if not running on a Xen control domain (dom0). It is not
+ * sufficient to check for the file to exist as any guest can mount
+ * xenfs to /proc/xen.
+ */
+status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output);
+if (status = 0)
+status = strncmp(output, control_d, 9);
+VIR_FREE(output);
+if (status) {
+VIR_INFO(No Xen capabilities detected, probably not running 
+ in a Xen Dom0.  Disabling libxenlight driver);
+
+return ret;
+}
+} else if (!virFileExists(HYPERVISOR_XENSTORED)) {
+VIR_INFO(Disabling driver as neither  HYPERVISOR_CAPABILITIES
+  nor  HYPERVISOR_CAPABILITIES  exist);


s/HYPERVISOR_CAPABILITIES/HYPERVISOR_XENSTORED/

ACK with that changed.


For the record, I tested this on Linux.  Looks good with Martin's comment 
addressed.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Entering freeze for libvirt 1.2.16

2015-05-27 Thread Daniel Veillard
  I'm one day late but I have tagged the release candidate 1
in git and pushed signed tarballs and rpms to the usual place:

  ftp://libvirt.org/libvirt/

 we have a bit less than 160 commits so far since 1.2.15.
I tried it with my usual limited testing and it looks fine,
https://ci.centos.org/ seems fine when it comes to libvirt,
but please give it a try so we can get potential bugs or
portability issues out.

 I will probably make an rc2 on Friday and push the final
release next Monday or Tuesday,

  thanks !

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] conf: improve the address check for dimm type

2015-05-27 Thread Peter Krempa
On Wed, May 27, 2015 at 14:39:58 +0800, Luyao Huang wrote:
 When hot-plug/cold-plug a memory device, we use
 memcmp() function to check if there is a memory device
 have the same address with the memory device we want
 hot-pluged. But qemu forbid use/hot-plug 2 memory device
 with same slot *or* the same base(qemu side this elemnt
 named addr).
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  src/conf/domain_conf.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 6e57425..413f839 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -3089,7 +3089,10 @@ virDomainDeviceInfoAddressIsEqual(const 
 virDomainDeviceInfo *a,
  break;
  
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
 -if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm)))
 +if (a-addr.dimm.slot != b-addr.dimm.slot 
 +(a-addr.dimm.base == 0 ||
 + b-addr.dimm.base == 0 ||
 + a-addr.dimm.base != b-addr.dimm.base))
  return false;

This function is designed to check if the address is equal not if it is
not conflicting for a particular hypervisor.

If you are going to enforce that both the address and base are
different, this function is not the right place.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] conf: improve the address check for dimm type

2015-05-27 Thread Luyao Huang
When hot-plug/cold-plug a memory device, we use
memcmp() function to check if there is a memory device
have the same address with the memory device we want
hot-pluged. But qemu forbid use/hot-plug 2 memory device
with same slot *or* the same base(qemu side this elemnt
named addr).

Signed-off-by: Luyao Huang lhu...@redhat.com
---
 src/conf/domain_conf.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6e57425..413f839 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3089,7 +3089,10 @@ virDomainDeviceInfoAddressIsEqual(const 
virDomainDeviceInfo *a,
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
-if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm)))
+if (a-addr.dimm.slot != b-addr.dimm.slot 
+(a-addr.dimm.base == 0 ||
+ b-addr.dimm.base == 0 ||
+ a-addr.dimm.base != b-addr.dimm.base))
 return false;
 break;
 }
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] conf: improve the address check for dimm type

2015-05-27 Thread lhuang


On 05/27/2015 03:03 PM, Peter Krempa wrote:

On Wed, May 27, 2015 at 14:39:58 +0800, Luyao Huang wrote:

When hot-plug/cold-plug a memory device, we use
memcmp() function to check if there is a memory device
have the same address with the memory device we want
hot-pluged. But qemu forbid use/hot-plug 2 memory device
with same slot *or* the same base(qemu side this elemnt
named addr).

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/conf/domain_conf.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6e57425..413f839 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3089,7 +3089,10 @@ virDomainDeviceInfoAddressIsEqual(const 
virDomainDeviceInfo *a,
  break;
  
  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:

-if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm)))
+if (a-addr.dimm.slot != b-addr.dimm.slot 
+(a-addr.dimm.base == 0 ||
+ b-addr.dimm.base == 0 ||
+ a-addr.dimm.base != b-addr.dimm.base))
  return false;

This function is designed to check if the address is equal not if it is
not conflicting for a particular hypervisor.

If you are going to enforce that both the address and base are
different, this function is not the right place.


Okay, reasonable to me, i will found a place for the dimm address check 
in src/qemu/*


Thanks you advise and quick review.



Peter


Luyao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] qemu: Fix compilation error when enum variable size differs from 'int'

2015-05-27 Thread Peter Krempa
On Tue, May 26, 2015 at 09:04:09 -0600, Eric Blake wrote:
 On 05/26/2015 02:31 AM, Peter Krempa wrote:
  Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
  via a pointer rather than in the return value. The change triggered
  problems with platforms where the compiler decides to use a data type of
  size different than integer at the point where we typecast it.
  
  Work around the issue by using an intermediate variable of the correct
  type that gets casted back by the default typecasting rules.
  ---
  Version 2:
  - removed block from the case statement
  - added ignore value section to silence coverity and initialized the temp 
  variable
  
   src/qemu/qemu_driver.c | 7 ---
   1 file changed, 4 insertions(+), 3 deletions(-)
 
 ACK

Pushed; Thanks.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: improve the sysinfo element XML format

2015-05-27 Thread lhuang


On 05/27/2015 07:59 AM, John Ferlan wrote:


On 05/22/2015 05:26 AM, Luyao Huang wrote:

When set sysinfo element without sub-elements,
libvirt will format it as:

   sysinfo type='smbios'
   /sysinfo

After improve the format:

   sysinfo type='smbios'/

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/util/virsysinfo.c | 33 ++---
  1 file changed, 22 insertions(+), 11 deletions(-)


ACK - I'll slightly adjust commit message before pushing


Thanks for your quick review



John


Luyao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] conf: do not format redirfilter element if it do not have sub-element

2015-05-27 Thread lhuang


On 05/27/2015 08:00 AM, John Ferlan wrote:


On 05/22/2015 05:26 AM, Luyao Huang wrote:

When set a redirfilter element without sub-element, libvirt will
format it like this:

 redirfilter
 /redirfilter

Just drop this element if it do not have any sub-element.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/conf/domain_conf.c | 4 
  1 file changed, 4 insertions(+)


ACK - I'll slightly adjust commit message before pushing


Thank you John :)



John


Luyao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list