Re: [libvirt] [PATCH] storage: Disallow wiping an extended disk partition

2015-06-15 Thread Michal Privoznik
On 10.06.2015 00:19, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1225694
 
 Check if the disk partition to be wiped is the extended partition, if
 so then disallow it. Do this via changing the wipeVol backend to check
 the volume before passing to the common virStorageBackendVolWipeLocal
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_disk.c | 20 +++-
  1 file changed, 19 insertions(+), 1 deletion(-)
 
 diff --git a/src/storage/storage_backend_disk.c 
 b/src/storage/storage_backend_disk.c
 index c4bd6fe..a283a86 100644
 --- a/src/storage/storage_backend_disk.c
 +++ b/src/storage/storage_backend_disk.c
 @@ -851,6 +851,24 @@ virStorageBackendDiskBuildVolFrom(virConnectPtr conn,
  }
  
  
 +static int
 +virStorageBackendDiskVolWipe(virConnectPtr conn,
 + virStoragePoolObjPtr pool,
 + virStorageVolDefPtr vol,
 + unsigned int algorithm,
 + unsigned int flags)
 +{
 +if (vol-source.partType != VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
 +return virStorageBackendVolWipeLocal(conn, pool, vol, algorithm, 
 flags);
 +
 +/* Wiping an extended partition is not support */
 +virReportError(VIR_ERR_NO_SUPPORT,
 +   _(cannot wipe extended partition '%s'),
 +   vol-target.path);
 +return -1;
 +}
 +
 +
  virStorageBackend virStorageBackendDisk = {
  .type = VIR_STORAGE_POOL_DISK,
  
 @@ -862,5 +880,5 @@ virStorageBackend virStorageBackendDisk = {
  .buildVolFrom = virStorageBackendDiskBuildVolFrom,
  .uploadVol = virStorageBackendVolUploadLocal,
  .downloadVol = virStorageBackendVolDownloadLocal,
 -.wipeVol = virStorageBackendVolWipeLocal,
 +.wipeVol = virStorageBackendDiskVolWipe,
  };
 

Correct, mangling partition table is not desired. BTW: couldn't
something similar happen with LVM?

ACK to this though.

Michal

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


Re: [libvirt] [PATCH] scsi: Adjust return status from getBlockDevice

2015-06-15 Thread Michal Privoznik
On 12.06.2015 13:22, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1224233
 
 Currently it's not possible to determine the difference between a
 fatal memory allocation or failure to open/read the directory error
 with a perhaps less fatal, I didn't find the block device in the
 directory (which may be a disk entry without a block device).
 
 In the case of the latter, we shouldn't cause failure to continue
 searching in the caller (virStorageBackendSCSIFindLUs), rather we
 should allow trying reading the next directory entry.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_scsi.c | 20 ++--
  1 file changed, 18 insertions(+), 2 deletions(-)
 
 diff --git a/src/storage/storage_backend_scsi.c 
 b/src/storage/storage_backend_scsi.c
 index e6c8bb5..c5105ec 100644
 --- a/src/storage/storage_backend_scsi.c
 +++ b/src/storage/storage_backend_scsi.c
 @@ -329,6 +329,15 @@ getOldStyleBlockDevice(const char *lun_path 
 ATTRIBUTE_UNUSED,
  }
  
  
 +/*
 + * Search a device entry for the block file
 + *
 + * Returns
 + *
 + *   0 = Found it
 + *   -1 = Fatal error
 + *   -2 = Didn't find in lun_path directory
 + */
  static int
  getBlockDevice(uint32_t host,
 uint32_t bus,
 @@ -354,6 +363,10 @@ getBlockDevice(uint32_t host,
  goto out;
  }
  
 +/* As long as virDirRead doesn't fail, if we fail to find the
 + * block file in this directory, allow caller to continue
 + */
 +retval = -2;
  while ((direrr = virDirRead(lun_dir, lun_dirent, lun_path))  0) {
  if (STREQLEN(lun_dirent-d_name, block, 5)) {
  if (strlen(lun_dirent-d_name) == 5) {
 @@ -368,6 +381,9 @@ getBlockDevice(uint32_t host,
  break;
  }
  }
 +/* Keep retval = -2 unless there was a fatal error in virDirRead */
 +if (retval == -2  direrr  0)
 +retval = -1;
  
  closedir(lun_dir);
  
 @@ -417,9 +433,9 @@ processLU(virStoragePoolObjPtr pool,
  VIR_DEBUG(%u:%u:%u:%u is a Direct-Access LUN,
host, bus, target, lun);
  
 -if (getBlockDevice(host, bus, target, lun, block_device)  0) {
 +if ((retval = getBlockDevice(host, bus, target, lun, block_device))  
 0) {
  VIR_DEBUG(Failed to find block device for this LUN);
 -return -1;
 +return retval;
  }
  
  retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun,
 

While this would work, I think if we follow out the pattern from the
rest of our functions, we can do better. Let me propose v2 to express
what I have in mind.

Michal

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


Re: [libvirt] [PATCH] storage: Disallow wiping an extended disk partition

2015-06-15 Thread John Ferlan


On 06/15/2015 06:12 AM, Michal Privoznik wrote:
 On 10.06.2015 00:19, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1225694

 Check if the disk partition to be wiped is the extended partition, if
 so then disallow it. Do this via changing the wipeVol backend to check
 the volume before passing to the common virStorageBackendVolWipeLocal

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_disk.c | 20 +++-
  1 file changed, 19 insertions(+), 1 deletion(-)

 diff --git a/src/storage/storage_backend_disk.c 
 b/src/storage/storage_backend_disk.c
 index c4bd6fe..a283a86 100644
 --- a/src/storage/storage_backend_disk.c
 +++ b/src/storage/storage_backend_disk.c
 @@ -851,6 +851,24 @@ virStorageBackendDiskBuildVolFrom(virConnectPtr conn,
  }
  
  
 +static int
 +virStorageBackendDiskVolWipe(virConnectPtr conn,
 + virStoragePoolObjPtr pool,
 + virStorageVolDefPtr vol,
 + unsigned int algorithm,
 + unsigned int flags)
 +{
 +if (vol-source.partType != VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
 +return virStorageBackendVolWipeLocal(conn, pool, vol, algorithm, 
 flags);
 +
 +/* Wiping an extended partition is not support */
 +virReportError(VIR_ERR_NO_SUPPORT,
 +   _(cannot wipe extended partition '%s'),
 +   vol-target.path);
 +return -1;
 +}
 +
 +
  virStorageBackend virStorageBackendDisk = {
  .type = VIR_STORAGE_POOL_DISK,
  
 @@ -862,5 +880,5 @@ virStorageBackend virStorageBackendDisk = {
  .buildVolFrom = virStorageBackendDiskBuildVolFrom,
  .uploadVol = virStorageBackendVolUploadLocal,
  .downloadVol = virStorageBackendVolDownloadLocal,
 -.wipeVol = virStorageBackendVolWipeLocal,
 +.wipeVol = virStorageBackendDiskVolWipe,
  };

 
 Correct, mangling partition table is not desired. BTW: couldn't
 something similar happen with LVM?
 

To a degree this was taken from what for the logical backend handling of
a sparse logical volume due to metadata overwrite where yes we were
overwriting metadata...  I think for the lv's we rely a bit more on lvm
to manage the partition and don't have the same issue about knowing
whether it's a primary, extended, or logical partition.

John

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


Re: [libvirt] [PATCH] qemu caps: spell queue

2015-06-15 Thread Michal Privoznik
On 15.06.2015 10:58, Daniel P. Berrange wrote:
 On Mon, Jun 15, 2015 at 10:49:06AM +0200, Ján Tomko wrote:
 ---
  src/qemu/qemu_capabilities.c | 4 ++--
  src/qemu/qemu_capabilities.h | 2 +-
  src/qemu/qemu_command.c  | 2 +-
  tests/qemuxml2argvtest.c | 2 +-
  4 files changed, 5 insertions(+), 5 deletions(-)

 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
 index 8a64422..27a632a 100644
 --- a/src/qemu/qemu_capabilities.c
 +++ b/src/qemu/qemu_capabilities.c
 @@ -286,7 +286,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
pci-serial,
aarch64-off,
  
 -  vhost-user-multiq, /* 190 */
 +  vhost-user-multiqueue, /* 190 */
  );
 
 Have we done a release that includes this ?  If so, we must not change
 it as it would cause failure to load existing running guest XML.
 

Fortunately no. The commit introducing the capability was pushed just a
week ago, after the release. If we want to do this, we must do it now.

ACK to the patch.

Michal

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

Re: [libvirt] [PATCH] qemu: Do not support 'serial' scsi-block 'lun' devices

2015-06-15 Thread Michal Privoznik
On 09.06.2015 22:54, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1021480
 
 Seems the property has been deprecated for qemu, although seemingly ignored.
 
 This patch enforces from a libvirt perspective that a scsi-block 'lun'
 device should not provide the 'serial' property.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/formatdomain.html.in | 3 +++
  src/qemu/qemu_command.c   | 7 +++
  2 files changed, 10 insertions(+)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 1781996..4eb907d 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -2502,6 +2502,9 @@
ddIf present, this specify serial number of virtual hard drive.
For example, it may look
like codelt;serialgt;WD-WMAP9A966149lt;/serialgt;/code.
 +  Not supported for scsi-block devices, that is those using
 +  disk codetype/code 'block' using codedevice/code 'lun'
 +  on codebus/code 'scsi'.
span class=sinceSince 0.7.1/span
/dd
dtcodewwn/code/dt
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 0a6d92f..cbea0d5 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3731,6 +3731,13 @@ qemuBuildDriveStr(virConnectPtr conn,
  virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) {
  if (qemuSafeSerialParamValue(disk-serial)  0)
  goto error;
 +if (disk-bus == VIR_DOMAIN_DISK_BUS_SCSI 
 +disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(scsi-block 'lun' devices do not support the 
 + serial property));
 +goto error;
 +}
  virBufferAddLit(opt, ,serial=);
  virBufferEscape(opt, '\\',  , %s, disk-serial);
  }
 


ACK

Michal

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


Re: [libvirt] [PATCH] qemu: fix cannot attach a virtio channel

2015-06-15 Thread lhuang


On 06/15/2015 04:25 PM, Ján Tomko wrote:

I'd rather be more specific in the commit summary, e.g.:
fix address allocation on chardev hotplug


good summary, thanks for you help


On Wed, Jun 10, 2015 at 10:49:37PM +0800, Luyao Huang wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1230039

When attach a channel which target type is virtio, we will
get an error:

error: Failed to attach device from channel-file.xml
error: internal error: virtio serial device has invalid address type

This issue was introduced in commit 9807c4.
We didn't check the chr device type is serial then check
if the device target type is pci-serial, but not all the
Chr device is a serial type so we should check the device
type before check the target type to avoid assign wrong
address to other device type chr device.

Also most of chr device do not need {pci, virtio-serial} address
in qemu, we just get the address for the device which needed.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/qemu/qemu_hotplug.c | 72 +++--
  1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3562de6..4d60513 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1531,6 +1531,48 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
  return ret;
  }
  
+static int

+qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
+virDomainChrDefPtr chr)
+{
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO 
+virDomainVirtioSerialAddrAutoAssign(NULL, priv-vioserialaddrs,
+chr-info, true)  0)
+return -1;
+
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL 
+chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI 
+(chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) 

Do we need to check the address type here? pci-serial should always have
a PCI address.


Yes, pci-serial should always have a PCI address, and add this check
is avoid always assign a pci address even we already specified the address
which type is not pci, in that case i think output an error will be better.
(although it is a corner case, and i notice when we start a guest with wrong
address type pci-serial will get the error,  but cannot get the error 
when attach

it, so i guess QE will complain about this ;) )

and after add this check the error will be:

# virsh attach-device rhel7.0  pci-serial.xml
error: Failed to attach device from pci-serial.xml
error: unsupported configuration: pci-serial requires address of pci type


+virDomainPCIAddressEnsureAddr(priv-pciaddrs, chr-info)  0)
+return -1;
+
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL 
+chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO 
+virDomainVirtioSerialAddrAutoAssign(NULL, priv-vioserialaddrs,
+chr-info, false)  0)
+return -1;
+
+return 0;
+}
+
+static void
+qemuDomainAttachChrDeviceReleaseAddr(qemuDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainChrDefPtr chr)
+{
+if ((chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
+ chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) ||
+(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL 
+ chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))
+virDomainVirtioSerialAddrRelease(priv-vioserialaddrs, chr-info);
+
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL 
+chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI)
+qemuDomainReleaseDeviceAddress(vm, chr-info, NULL);
+}
+
  int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr)
@@ -1541,7 +1583,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  char *devstr = NULL;
  char *charAlias = NULL;
  bool need_release = false;
-bool allowZero = false;
  
  if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {

  virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -1552,22 +1593,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  if (qemuAssignDeviceChrAlias(vmdef, chr, -1)  0)
  goto cleanup;
  
-if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 

-chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
-allowZero = true;
-
-if (chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
-if (virDomainPCIAddressEnsureAddr(priv-pciaddrs, chr-info)  0)
-goto cleanup;
-} else if (chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
-

Re: [libvirt] [PATCH] configure: Remove check for pkcheck_supports_uid

2015-06-15 Thread Martin Kletzander

On Fri, Jun 05, 2015 at 01:01:34PM +0200, Guido Günther wrote:

We're using Polkit's DBus API so no need to check wether this feature is
supported. We don't use the result or the path to the pkcheck program
anywhere.
---
configure.ac | 9 -
1 file changed, 9 deletions(-)



ACK, this is not needed anymore.


diff --git a/configure.ac b/configure.ac
index abf4436..073624b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1362,15 +1362,6 @@ if test x$with_polkit = xyes || test x$with_polkit = 
xcheck; then
  dnl Check for new polkit first - just a binary
  AC_PATH_PROG([PKCHECK_PATH],[pkcheck], [], [/usr/sbin:$PATH])
  if test x$PKCHECK_PATH != x ; then
-AC_DEFINE_UNQUOTED([PKCHECK_PATH],[$PKCHECK_PATH],[Location of pkcheck 
program])
-AC_MSG_CHECKING([whether pkcheck supports uid value])
-pkcheck_supports_uid=`$PKG_CONFIG --variable pkcheck_supports_uid 
polkit-gobject-1`
-if test x$pkcheck_supports_uid = xtrue; then
-  AC_MSG_RESULT([yes])
-  AC_DEFINE_UNQUOTED([PKCHECK_SUPPORTS_UID], 1, [Pass uid to pkcheck])
-else
-  AC_MSG_RESULT([no])
-fi
AC_DEFINE_UNQUOTED([WITH_POLKIT], 1,
[use PolicyKit for UNIX socket access checks])
AC_DEFINE_UNQUOTED([WITH_POLKIT1], 1,
--
2.1.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] qemu caps: spell queue

2015-06-15 Thread Daniel P. Berrange
On Mon, Jun 15, 2015 at 10:49:06AM +0200, Ján Tomko wrote:
 ---
  src/qemu/qemu_capabilities.c | 4 ++--
  src/qemu/qemu_capabilities.h | 2 +-
  src/qemu/qemu_command.c  | 2 +-
  tests/qemuxml2argvtest.c | 2 +-
  4 files changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
 index 8a64422..27a632a 100644
 --- a/src/qemu/qemu_capabilities.c
 +++ b/src/qemu/qemu_capabilities.c
 @@ -286,7 +286,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
pci-serial,
aarch64-off,
  
 -  vhost-user-multiq, /* 190 */
 +  vhost-user-multiqueue, /* 190 */
  );

Have we done a release that includes this ?  If so, we must not change
it as it would cause failure to load existing running guest XML.


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] virfile: Report useful error fork approach to create NFS mount point fails

2015-06-15 Thread Ján Tomko
On Thu, Jun 11, 2015 at 11:15:17AM +0200, Erik Skultety wrote:
 Commit 92d9114e tweaked the way we handle child errors when using fork
 approach to set specific permissions. The same logic should be used to
 create directories with specified permissions as well, otherwise the parent
 process doesn't report any useful error unknown cause while still returning
 negative errcode.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1230137
 ---
  src/util/virfile.c | 48 +++-
  1 file changed, 39 insertions(+), 9 deletions(-)
 
 diff --git a/src/util/virfile.c b/src/util/virfile.c
 index 5ff4668..7675eeb 100644
 --- a/src/util/virfile.c
 +++ b/src/util/virfile.c
 @@ -2376,6 +2376,7 @@ virDirCreate(const char *path,
  if (pid) { /* parent */
  /* wait for child to complete, and retrieve its exit code */
  VIR_FREE(groups);
 +
  while ((waitret = waitpid(pid, status, 0)) == -1  errno == EINTR);
  if (waitret == -1) {
  ret = -errno;
 @@ -2384,11 +2385,33 @@ virDirCreate(const char *path,
   path);
  goto parenterror;
  }
 -if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES) {

The old condition was wrong:
child would call _exit(-EACCESS) and we would the compare -(-EACCESS) to 
-EACCESS

 -/* fall back to the simpler method, which works better in
 - * some cases */
 -return virDirCreateNoFork(path, mode, uid, gid, flags);
 +
 +/*
 + * If waitpid succeeded, but if the child exited abnormally or
 + * reported non-zero status, report failure, except for EACCES where
 + * we try to fall back to non-fork method as in the original logic.
 + */

What is the original logic referenced here?

 +if (!WIFEXITED(status) || (WEXITSTATUS(status)) != 0) {
 +if (WEXITSTATUS(status) == EACCES)
 +return virDirCreateNoFork(path, mode, uid, gid, flags);
 +char *msg = virProcessTranslateStatus(status);
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(child failed to create '%s': %s),
 +   path, msg);
 +VIR_FREE(msg);
 +/* Use child exit status if possible; otherwise,
 + * just use -EACCES, since by our original failure in
 + * the non fork+setuid path would have been EACCES or
 + * EPERM by definition (see qemuOpenFileAs after the
 + * first virFileOpenAs failure), but EACCES is close enough.

This comment references irrelevant functions and seems redundant.

 + * Besides -EPERM is like returning fd == -1.
 + */
 +if (WIFEXITED(status))
 +ret = -WEXITSTATUS(status);
 +else
 +ret = -EACCES;
  }
 +
   parenterror:
  return ret;
  }
 @@ -2400,15 +2423,14 @@ virDirCreate(const char *path,
  ret = -errno;
  goto childerror;
  }
 +
  if (mkdir(path, mode)  0) {
  ret = -errno;

 -if (ret != -EACCES) {
 -/* in case of EACCES, the parent will retry */
 -virReportSystemError(errno, _(child failed to create directory 
 '%s'),
 - path);
 -}
 +virReportSystemError(errno, _(child failed to create directory 
 '%s'),
 + path);

Do we really need this hunk?
If we fail with EACCES, parent should call the NoFork function which
should return success / report error.

  goto childerror;
  }
 +

The space ajdustments would be better in a separate patch.

  /* check if group was set properly by creating after
   * setgid. If not, try doing it with chown */
  if (stat(path, st) == -1) {
 @@ -2417,6 +2439,7 @@ virDirCreate(const char *path,
   _(stat of '%s' failed), path);
  goto childerror;
  }
 +
  if ((st.st_gid != gid)  (chown(path, (uid_t) -1, gid)  0)) {
  ret = -errno;
  virReportSystemError(errno,
 @@ -2424,13 +2447,20 @@ virDirCreate(const char *path,
   path, (unsigned int) gid);
  goto childerror;
  }
 +
  if (mode != (mode_t) -1  chmod(path, mode)  0) {
  virReportSystemError(errno,
   _(cannot set mode of '%s' to %04o),
   path, mode);
  goto childerror;
  }
 +
   childerror:
 +ret = -ret;
 +if ((ret  0xff) != ret) {
 +VIR_WARN(unable to pass desired return value %d, ret);
 +ret = 0xff;
 +}

And flipping the return value to unsigned should be separate from adding
the new error message in the parent.

Jan


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

[libvirt] [PATCH] qemu caps: spell queue

2015-06-15 Thread Ján Tomko
---
 src/qemu/qemu_capabilities.c | 4 ++--
 src/qemu/qemu_capabilities.h | 2 +-
 src/qemu/qemu_command.c  | 2 +-
 tests/qemuxml2argvtest.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8a64422..27a632a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -286,7 +286,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   pci-serial,
   aarch64-off,
 
-  vhost-user-multiq, /* 190 */
+  vhost-user-multiqueue, /* 190 */
 );
 
 
@@ -3317,7 +3317,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 /* vhost-user supports multi-queue from v2.4.0 onwards,
  * but there is no way to query for that capability */
 if (qemuCaps-version = 2004000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQ);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
 
 if (virQEMUCapsProbeQMPCommands(qemuCaps, mon)  0)
 goto cleanup;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3c166b6..30aa504 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -229,7 +229,7 @@ typedef enum {
 QEMU_CAPS_DEA_KEY_WRAP   = 187, /* -machine dea_key_wrap */
 QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
 QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */
-QEMU_CAPS_VHOSTUSER_MULTIQ   = 190, /* vhost-user with -netdev queues= */
+QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 89f775d..9f247de 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8141,7 +8141,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
   net-info.alias, net-info.alias);
 
 if (queues  1) {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQ)) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(multi-queue is not supported for vhost-user 
  with this QEMU binary));
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 35bfd29..a90f9a6 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -981,7 +981,7 @@ mymain(void)
 DO_TEST_PARSE_ERROR(vhost_queues-invalid, NONE);
 DO_TEST(net-vhostuser, QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV);
 DO_TEST(net-vhostuser-multiq,
-QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV, QEMU_CAPS_VHOSTUSER_MULTIQ);
+QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV, 
QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
 DO_TEST_FAILURE(net-vhostuser-multiq, QEMU_CAPS_DEVICE, 
QEMU_CAPS_NETDEV);
 DO_TEST(net-user, NONE);
 DO_TEST(net-virtio, NONE);
-- 
2.3.6

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


[libvirt] [PATCH 3/3] scsi: Adjust return status from getBlockDevice

2015-06-15 Thread Michal Privoznik
From: John Ferlan jfer...@redhat.com

https://bugzilla.redhat.com/show_bug.cgi?id=1224233

Currently it's not possible to determine the difference between a
fatal memory allocation or failure to open/read the directory error
with a perhaps less fatal, I didn't find the block device in the
directory (which may be a disk entry without a block device).

In the case of the latter, we shouldn't cause failure to continue
searching in the caller (virStorageBackendSCSIFindLUs), rather we
should allow trying reading the next directory entry.

Signed-off-by: John Ferlan jfer...@redhat.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/storage/storage_backend_scsi.c | 48 +++---
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c 
b/src/storage/storage_backend_scsi.c
index ddfbade..b426145 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -324,6 +324,15 @@ getOldStyleBlockDevice(const char *lun_path 
ATTRIBUTE_UNUSED,
 }
 
 
+/*
+ * Search a device entry for the block file
+ *
+ * Returns
+ *
+ *   0 = Found it
+ *   -1 = Fatal error
+ *   -2 = Didn't find in lun_path directory
+ */
 static int
 getBlockDevice(uint32_t host,
uint32_t bus,
@@ -337,36 +346,47 @@ getBlockDevice(uint32_t host,
 int retval = -1;
 int direrr;
 
+*block_device = NULL;
+
 if (virAsprintf(lun_path, /sys/bus/scsi/devices/%u:%u:%u:%u,
 host, bus, target, lun)  0)
-goto out;
+goto cleanup;
 
-lun_dir = opendir(lun_path);
-if (lun_dir == NULL) {
+if (!(lun_dir = opendir(lun_path))) {
 virReportSystemError(errno,
  _(Failed to opendir sysfs path '%s'),
  lun_path);
-goto out;
+goto cleanup;
 }
 
 while ((direrr = virDirRead(lun_dir, lun_dirent, lun_path))  0) {
 if (STREQLEN(lun_dirent-d_name, block, 5)) {
 if (strlen(lun_dirent-d_name) == 5) {
-retval = getNewStyleBlockDevice(lun_path,
-lun_dirent-d_name,
-block_device);
+if (getNewStyleBlockDevice(lun_path,
+   lun_dirent-d_name,
+   block_device)  0)
+goto cleanup;
 } else {
-retval = getOldStyleBlockDevice(lun_path,
-lun_dirent-d_name,
-block_device);
+if (getOldStyleBlockDevice(lun_path,
+   lun_dirent-d_name,
+   block_device)  0)
+goto cleanup;
 }
 break;
 }
 }
+if (direrr  0)
+goto cleanup;
+if (!*block_device) {
+retval = -2;
+goto cleanup;
+}
 
-closedir(lun_dir);
+retval = 0;
 
- out:
+ cleanup:
+if (lun_dir)
+closedir(lun_dir);
 VIR_FREE(lun_path);
 return retval;
 }
@@ -412,9 +432,9 @@ processLU(virStoragePoolObjPtr pool,
 VIR_DEBUG(%u:%u:%u:%u is a Direct-Access LUN,
   host, bus, target, lun);
 
-if (getBlockDevice(host, bus, target, lun, block_device)  0) {
+if ((retval = getBlockDevice(host, bus, target, lun, block_device))  0) {
 VIR_DEBUG(Failed to find block device for this LUN);
-return -1;
+return retval;
 }
 
 retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun,
-- 
2.3.6

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


[libvirt] [PATCH 2/3] getOldStyleBlockDevice: Adjust formatting

2015-06-15 Thread Michal Privoznik
Instead of initializing return value to zero (success) and overwriting
it on every failure just before the control jumps onto 'out' label,
let's initialize to an error value and set to zero only when we are
sure about the success. Just follow the pattern we have in the rest of
the code.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/storage/storage_backend_scsi.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c 
b/src/storage/storage_backend_scsi.c
index 3c1bae6..ddfbade 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -301,27 +301,25 @@ getOldStyleBlockDevice(const char *lun_path 
ATTRIBUTE_UNUSED,
char **block_device)
 {
 char *blockp = NULL;
-int retval = 0;
+int retval = -1;
 
 /* old-style; just parse out the sd */
-blockp = strrchr(block_name, ':');
-if (blockp == NULL) {
+if (!(blockp = strrchr(block_name, ':'))) {
 /* Hm, wasn't what we were expecting; have to give up */
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to parse block name %s),
block_name);
-retval = -1;
+goto cleanup;
 } else {
 blockp++;
-if (VIR_STRDUP(*block_device, blockp)  0) {
-retval = -1;
-goto out;
-}
+if (VIR_STRDUP(*block_device, blockp)  0)
+goto cleanup;
 
 VIR_DEBUG(Block device is '%s', *block_device);
 }
 
- out:
+retval = 0;
+ cleanup:
 return retval;
 }
 
-- 
2.3.6

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


[libvirt] [PATCH 0/3] Couple of storage_backend_scsi.c fixes

2015-06-15 Thread Michal Privoznik
As promised here [1] I'm proposing my own patch to fix the problem referenced
here [2].

1: https://www.redhat.com/archives/libvir-list/2015-June/msg00621.html

2: https://www.redhat.com/archives/libvir-list/2015-June/msg00567.html


John Ferlan (1):
  scsi: Adjust return status from getBlockDevice

Michal Privoznik (2):
  getNewStyleBlockDevice: Adjust formatting
  getOldStyleBlockDevice: Adjust formatting

 src/storage/storage_backend_scsi.c | 91 ++
 1 file changed, 53 insertions(+), 38 deletions(-)

-- 
2.3.6

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


Re: [libvirt] [PATCH v2] storage: Need to set secrettype for direct iscsi disk volume

2015-06-15 Thread Michal Privoznik
On 09.06.2015 18:03, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1200206
 
 Commit id '1b4eaa61' added the ability to have a mode='direct' for
 an iscsi disk volume.  It relied on virStorageTranslateDiskSourcePool
 in order to copy any disk source pool authentication information to
 the direct disk volume, but it neglected to also copy the 'secrettype'
 field which ends up being used in the domain volume formatting code.
 Adding a secrettype for this case will allow for proper formatting later
 and allow disk snapshotting to work properly
 
 Additionally libvirtd restart processing would fail to find the domain
 since the translation processing code is run after domain xml processing,
 so handle the the case where the authdef could have an empty secrettype
 field when processing the auth and additionally ignore performing the
 actual and expected auth secret type checks for a DISK_VOLUME since that
 data will be reassembled later during translation processing of the
 running domain.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
 Changes since v1:
  
  - Found that the libvirtd restart path caused issues - my initial attempt
to fix made a bad assumption - that I was running with a domain started
after the initial patch which would have the secrettype filled in by
the translate code. 
 
So this patch changes the domain parse code to better account for the
chance that secrettype isn't yet provided in the XML by having the
authdef processing code fill in the value.
 
Secondly since the pooltype was only filled during the translation on
libvirtd restart and that occurs after domain xml process, the pooltype
field would be empty - thus it couldn't be used for comparison. Since,
translation processing will destroy the authdef read in at parse time,
modify the actual and expected check to ignore the DISK_VOLUME case
 
  src/conf/domain_conf.c   | 16 +++-
  src/storage/storage_driver.c | 10 ++
  2 files changed, 25 insertions(+), 1 deletion(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 36de844..d621c01 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -6571,6 +6571,16 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 xmlStrEqual(cur-name, BAD_CAST auth)) {
  if (!(authdef = virStorageAuthDefParse(node-doc, cur)))
  goto error;
 +/* Shared processing code with storage pools can leave
 + * this empty, but disk formatting uses it as does command
 + * creation - so use the secretType to attempt to fill it in.
 + */
 +if (!authdef-secrettype) {
 +const char *secrettype =
 +virSecretUsageTypeToString(authdef-secretType);
 +if (VIR_STRDUP(authdef-secrettype, secrettype)  0)
 +goto error;
 +}

So _virStorageAuthDef has both @secrettype and @secretType? That's
rather perplexing.

  if ((auth_secret_usage =
   virSecretUsageTypeFromString(authdef-secrettype))  0) 
 {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 @@ -6790,7 +6800,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
  cur = cur-next;
  }
  
 -if (auth_secret_usage != -1  auth_secret_usage != 
 expected_secret_usage) {
 +/* Disk volume types will have authentication information handled in
 + * virStorageTranslateDiskSourcePool
 + */
 +if (def-src-type != VIR_STORAGE_TYPE_VOLUME 
 +auth_secret_usage != -1  auth_secret_usage != 
 expected_secret_usage) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(invalid secret type '%s'),
 virSecretUsageTypeToString(auth_secret_usage));
 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
 index ab8675d..57060ab 100644
 --- a/src/storage/storage_driver.c
 +++ b/src/storage/storage_driver.c
 @@ -3310,6 +3310,16 @@ virStorageTranslateDiskSourcePool(virConnectPtr conn,
   pooldef-source)  0)
 goto cleanup;
  
 +   /* Source pool may not fill in the secrettype field,
 +* so we need to do so here
 +*/
 +   if (def-src-auth  !def-src-auth-secrettype) {
 +   const char *secrettype =
 +   virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI);
 +   if (VIR_STRDUP(def-src-auth-secrettype, secrettype)  0)
 +   goto cleanup;
 +   }
 +
 if (virStorageAddISCSIPoolSourceHost(def, pooldef)  0)
 goto cleanup;
 break;
 

But the patch is right. ACK.

Michal

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


Re: [libvirt] [PATCH v2] storage: Need to set secrettype for direct iscsi disk volume

2015-06-15 Thread John Ferlan


On 06/15/2015 05:44 AM, Michal Privoznik wrote:
 On 09.06.2015 18:03, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1200206

 Commit id '1b4eaa61' added the ability to have a mode='direct' for
 an iscsi disk volume.  It relied on virStorageTranslateDiskSourcePool
 in order to copy any disk source pool authentication information to
 the direct disk volume, but it neglected to also copy the 'secrettype'
 field which ends up being used in the domain volume formatting code.
 Adding a secrettype for this case will allow for proper formatting later
 and allow disk snapshotting to work properly

 Additionally libvirtd restart processing would fail to find the domain
 since the translation processing code is run after domain xml processing,
 so handle the the case where the authdef could have an empty secrettype
 field when processing the auth and additionally ignore performing the
 actual and expected auth secret type checks for a DISK_VOLUME since that
 data will be reassembled later during translation processing of the
 running domain.

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
 Changes since v1:
  
  - Found that the libvirtd restart path caused issues - my initial attempt
to fix made a bad assumption - that I was running with a domain started
after the initial patch which would have the secrettype filled in by
the translate code. 

So this patch changes the domain parse code to better account for the
chance that secrettype isn't yet provided in the XML by having the
authdef processing code fill in the value.

Secondly since the pooltype was only filled during the translation on
libvirtd restart and that occurs after domain xml process, the pooltype
field would be empty - thus it couldn't be used for comparison. Since,
translation processing will destroy the authdef read in at parse time,
modify the actual and expected check to ignore the DISK_VOLUME case

  src/conf/domain_conf.c   | 16 +++-
  src/storage/storage_driver.c | 10 ++
  2 files changed, 25 insertions(+), 1 deletion(-)

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 36de844..d621c01 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -6571,6 +6571,16 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 xmlStrEqual(cur-name, BAD_CAST auth)) {
  if (!(authdef = virStorageAuthDefParse(node-doc, cur)))
  goto error;
 +/* Shared processing code with storage pools can leave
 + * this empty, but disk formatting uses it as does command
 + * creation - so use the secretType to attempt to fill it 
 in.
 + */
 +if (!authdef-secrettype) {
 +const char *secrettype =
 +virSecretUsageTypeToString(authdef-secretType);
 +if (VIR_STRDUP(authdef-secrettype, secrettype)  0)
 +goto error;
 +}
 
 So _virStorageAuthDef has both @secrettype and @secretType? That's
 rather perplexing.
 

Yeah - I think it was a convenience perhaps when trying to combine the
various authdef functions that had existed.  I look to generate a
followup that removes the need for it

Tks -

John
  if ((auth_secret_usage =
   virSecretUsageTypeFromString(authdef-secrettype))  
 0) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 @@ -6790,7 +6800,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
  cur = cur-next;
  }
  
 -if (auth_secret_usage != -1  auth_secret_usage != 
 expected_secret_usage) {
 +/* Disk volume types will have authentication information handled in
 + * virStorageTranslateDiskSourcePool
 + */
 +if (def-src-type != VIR_STORAGE_TYPE_VOLUME 
 +auth_secret_usage != -1  auth_secret_usage != 
 expected_secret_usage) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(invalid secret type '%s'),
 virSecretUsageTypeToString(auth_secret_usage));
 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
 index ab8675d..57060ab 100644
 --- a/src/storage/storage_driver.c
 +++ b/src/storage/storage_driver.c
 @@ -3310,6 +3310,16 @@ virStorageTranslateDiskSourcePool(virConnectPtr conn,
   pooldef-source)  0)
 goto cleanup;
  
 +   /* Source pool may not fill in the secrettype field,
 +* so we need to do so here
 +*/
 +   if (def-src-auth  !def-src-auth-secrettype) {
 +   const char *secrettype =
 +   virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI);
 +   if (VIR_STRDUP(def-src-auth-secrettype, secrettype)  0)
 +   goto cleanup;
 +   }
 +
 if 

[libvirt] [PATCH 1/3] getNewStyleBlockDevice: Adjust formatting

2015-06-15 Thread Michal Privoznik
Instead of initializing return value to zero (success) and overwriting
it on every failure just before the control jumps onto 'out' label,
let's initialize to an error value and set to zero only when we are
sure about the success. Just follow the pattern we have in the rest of
the code.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/storage/storage_backend_scsi.c | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c 
b/src/storage/storage_backend_scsi.c
index e6c8bb5..3c1bae6 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -255,44 +255,41 @@ getNewStyleBlockDevice(const char *lun_path,
 char *block_path = NULL;
 DIR *block_dir = NULL;
 struct dirent *block_dirent = NULL;
-int retval = 0;
+int retval = -1;
 int direrr;
 
 if (virAsprintf(block_path, %s/block, lun_path)  0)
-goto out;
+goto cleanup;
 
 VIR_DEBUG(Looking for block device in '%s', block_path);
 
-block_dir = opendir(block_path);
-if (block_dir == NULL) {
+if (!(block_dir = opendir(block_path))) {
 virReportSystemError(errno,
  _(Failed to opendir sysfs path '%s'),
  block_path);
-retval = -1;
-goto out;
+goto cleanup;
 }
 
 while ((direrr = virDirRead(block_dir, block_dirent, block_path))  0) {
-
 if (STREQLEN(block_dirent-d_name, ., 1))
 continue;
 
-if (VIR_STRDUP(*block_device, block_dirent-d_name)  0) {
-closedir(block_dir);
-retval = -1;
-goto out;
-}
+if (VIR_STRDUP(*block_device, block_dirent-d_name)  0)
+goto cleanup;
 
 VIR_DEBUG(Block device is '%s', *block_device);
 
 break;
 }
+
 if (direrr  0)
-retval = -1;
+goto cleanup;
 
-closedir(block_dir);
+retval = 0;
 
- out:
+ cleanup:
+if (block_dir)
+closedir(block_dir);
 VIR_FREE(block_path);
 return retval;
 }
-- 
2.3.6

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


Re: [libvirt] [PATCH 0/3] Couple of storage_backend_scsi.c fixes

2015-06-15 Thread John Ferlan


On 06/15/2015 07:24 AM, Michal Privoznik wrote:
 As promised here [1] I'm proposing my own patch to fix the problem referenced
 here [2].
 
 1: https://www.redhat.com/archives/libvir-list/2015-June/msg00621.html
 
 2: https://www.redhat.com/archives/libvir-list/2015-June/msg00567.html
 
 
 John Ferlan (1):
   scsi: Adjust return status from getBlockDevice
 
 Michal Privoznik (2):
   getNewStyleBlockDevice: Adjust formatting
   getOldStyleBlockDevice: Adjust formatting
 
  src/storage/storage_backend_scsi.c | 91 
 ++
  1 file changed, 53 insertions(+), 38 deletions(-)
 

ACK to 1  2

John

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


[libvirt] [sandbox PATCH 4/4] Add host-image format parameter

2015-06-15 Thread Cédric Bosdonnat
Let the user specify the format of the source disk image in host-image
mounts. This will allow us to mount other image types than raw ones.
---
 .../libvirt-sandbox-builder-container.c|  4 +
 libvirt-sandbox/libvirt-sandbox-builder-machine.c  |  9 +++
 .../libvirt-sandbox-config-mount-host-image.c  | 91 +-
 .../libvirt-sandbox-config-mount-host-image.h  |  5 +-
 libvirt-sandbox/libvirt-sandbox-config.c   | 68 +++-
 libvirt-sandbox/libvirt-sandbox.sym|  5 ++
 libvirt-sandbox/tests/test-config.c|  1 +
 7 files changed, 175 insertions(+), 8 deletions(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c 
b/libvirt-sandbox/libvirt-sandbox-builder-container.c
index c3a58b2..383c956 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-container.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c
@@ -273,6 +273,8 @@ static gboolean 
gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil
 g_object_unref(fs);
 } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) {
 GVirSandboxConfigMountFile *mfile = 
GVIR_SANDBOX_CONFIG_MOUNT_FILE(mconfig);
+GVirSandboxConfigMountHostImage *mimage = 
GVIR_SANDBOX_CONFIG_MOUNT_HOST_IMAGE(mconfig);
+GVirConfigDomainDiskFormat format;
 
 fs = gvir_config_domain_filesys_new();
 gvir_config_domain_filesys_set_type(fs, 
GVIR_CONFIG_DOMAIN_FILESYS_FILE);
@@ -281,6 +283,8 @@ static gboolean 
gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil
   
gvir_sandbox_config_mount_file_get_source(mfile));
 gvir_config_domain_filesys_set_target(fs,
   
gvir_sandbox_config_mount_get_target(mconfig));
+format = gvir_sandbox_config_mount_host_image_get_format(mimage);
+gvir_config_domain_filesys_set_driver_format(fs, format);
 
 gvir_config_domain_add_device(domain,
   GVIR_CONFIG_DOMAIN_DEVICE(fs));
diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c 
b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
index e342ba1..5e6bf72 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
@@ -497,6 +497,7 @@ static gboolean 
gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde
 {
 GVirConfigDomainFilesys *fs;
 GVirConfigDomainDisk *disk;
+GVirConfigDomainDiskDriver *diskDriver;
 GVirConfigDomainInterface *iface;
 GVirConfigDomainMemballoon *ball;
 GVirConfigDomainConsole *con;
@@ -560,6 +561,8 @@ static gboolean 
gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde
 
 } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) {
 GVirSandboxConfigMountFile *mfile = 
GVIR_SANDBOX_CONFIG_MOUNT_FILE(mconfig);
+GVirSandboxConfigMountHostImage *mimage = 
GVIR_SANDBOX_CONFIG_MOUNT_HOST_IMAGE(mconfig);
+GVirConfigDomainDiskFormat format;
 gchar *target = g_strdup_printf(vd%c, (char)('a' + 
nHostImage++));
 
 disk = gvir_config_domain_disk_new();
@@ -568,8 +571,14 @@ static gboolean 
gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde

gvir_sandbox_config_mount_file_get_source(mfile));
 gvir_config_domain_disk_set_target_dev(disk, target);
 
+diskDriver = gvir_config_domain_disk_driver_new();
+format = gvir_sandbox_config_mount_host_image_get_format(mimage);
+gvir_config_domain_disk_driver_set_format(diskDriver, format);
+gvir_config_domain_disk_set_driver(disk, diskDriver);
+
 gvir_config_domain_add_device(domain,
   GVIR_CONFIG_DOMAIN_DEVICE(disk));
+g_object_unref(diskDriver);
 g_object_unref(disk);
 g_free(target);
 }
diff --git a/libvirt-sandbox/libvirt-sandbox-config-mount-host-image.c 
b/libvirt-sandbox/libvirt-sandbox-config-mount-host-image.c
index 61e8f42..37573ef 100644
--- a/libvirt-sandbox/libvirt-sandbox-config-mount-host-image.c
+++ b/libvirt-sandbox/libvirt-sandbox-config-mount-host-image.c
@@ -45,21 +45,90 @@
 
 struct _GVirSandboxConfigMountHostImagePrivate
 {
-gboolean unused;
+GVirConfigDomainDiskFormat format;
 };
 
 G_DEFINE_TYPE(GVirSandboxConfigMountHostImage, 
gvir_sandbox_config_mount_host_image, GVIR_SANDBOX_TYPE_CONFIG_MOUNT_FILE);
 
+enum {
+PROP_0,
+PROP_FORMAT,
+};
+
+enum {
+LAST_SIGNAL
+};
+
+//static gint signals[LAST_SIGNAL];
+
+
+static void gvir_sandbox_config_mount_host_image_get_property(GObject *object,
+  guint prop_id,
+  GValue 

Re: [libvirt] Libvirt bite sized tasks

2015-06-15 Thread Dan Mossor

On 06/15/2015 02:59 AM, Michal Privoznik wrote:

Dear lists,

I've just started new wiki page which aim is to summarize small and
trivial tasks, that starting contributors can take, investigate and
implement. The aim is to give them something easy to start with while
not scaring them out about complexity of our code. The page can be found
here:

http://wiki.libvirt.org/page/BiteSizedTasks

The name is copied from qemu wiki:

http://wiki.qemu.org/BiteSizedTasks

Please, feel free to extend the list. I plan to use it in the future
when interviewing GSoC candidates.

Michal

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



According to Laine [0], self-registration for the wiki is disabled. What 
would be the correct avenue of approach for those of us that are willing 
to contribute but don't have libvirt wiki accounts? Is there work in 
progress to tie it to FAS or OpenID?


[0]https://www.redhat.com/archives/libvir-list/2015-June/msg00584.html
--
Dan Mossor, RHCSA
Systems Engineer
Fedora Server WG | Fedora KDE WG | Fedora QA Team
Fedora Infrastructure Apprentice
FAS: dmossor IRC: danofsatx
San Antonio, Texas, USA

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


[libvirt] [sandbox PATCH 1/4] Make sure the sandbox state dir and config can be accessed

2015-06-15 Thread Cédric Bosdonnat
When running a KVM sandbox as root, the qemu process will run as
another user (likely qemu). We need to make sure this user can access
the vmlinux and initrd.img, sandbox.cfg and mounts.cfg files.
---
 libvirt-sandbox/libvirt-sandbox-config.c  | 2 +-
 libvirt-sandbox/libvirt-sandbox-context-interactive.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-config.c 
b/libvirt-sandbox/libvirt-sandbox-config.c
index 087b5ce..8991043 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -2258,7 +2258,7 @@ gboolean 
gvir_sandbox_config_save_to_path(GVirSandboxConfig *config,
 if (!(data = g_key_file_to_data(file, len, error)))
 goto cleanup;
 
-if (!(os = G_OUTPUT_STREAM(g_file_create(f, G_FILE_CREATE_PRIVATE, NULL, 
error
+if (!(os = G_OUTPUT_STREAM(g_file_create(f, G_FILE_CREATE_NONE, NULL, 
error
 goto cleanup;
 
 if (!g_output_stream_write_all(os, data, len, NULL, NULL, error))
diff --git a/libvirt-sandbox/libvirt-sandbox-context-interactive.c 
b/libvirt-sandbox/libvirt-sandbox-context-interactive.c
index cec7965..78b2fbd 100644
--- a/libvirt-sandbox/libvirt-sandbox-context-interactive.c
+++ b/libvirt-sandbox/libvirt-sandbox-context-interactive.c
@@ -217,8 +217,8 @@ static gboolean 
gvir_sandbox_context_interactive_start(GVirSandboxContext *ctxt,
 error)))
 goto cleanup;
 
-g_mkdir_with_parents(statedir, 0700);
-g_mkdir_with_parents(configdir, 0700);
+g_mkdir_with_parents(statedir, 0755);
+g_mkdir_with_parents(configdir, 0755);
 
 unlink(configfile);
 if (!gvir_sandbox_config_save_to_path(config, configfile, error))
-- 
2.1.4

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


[libvirt] [sandbox PATCH 0/4] Getting qemu sandboxes run as root + host-image format

2015-06-15 Thread Cédric Bosdonnat
Hi all,

Here are a few patches to make sandboxes run with qemu:///system connection.
The last one is just a new feature to allow using somethings else than RAW 
images
in host-image mounts. This feature will later be needed to run docker container
using Eren's work.

Cédric Bosdonnat (4):
  Make sure the sandbox state dir and config can be accessed
  Write /dev/vd* instead of vd* in mounts.cfg
  qemu: mount all host-images as ext4
  Add host-image format parameter

 .../libvirt-sandbox-builder-container.c|  4 +
 libvirt-sandbox/libvirt-sandbox-builder-machine.c  | 13 +++-
 .../libvirt-sandbox-config-mount-host-image.c  | 91 +-
 .../libvirt-sandbox-config-mount-host-image.h  |  5 +-
 libvirt-sandbox/libvirt-sandbox-config.c   | 70 +++--
 .../libvirt-sandbox-context-interactive.c  |  4 +-
 libvirt-sandbox/libvirt-sandbox.sym|  5 ++
 libvirt-sandbox/tests/test-config.c|  1 +
 8 files changed, 180 insertions(+), 13 deletions(-)

-- 
2.1.4

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

[libvirt] [PATCH] qemu:lxc:xml:libxl:xen: improve the error in openconsole/channel

2015-06-15 Thread Luyao Huang
We allow do not pass the dev_name to openconsole() and openchannel()
function, but the error message is not good when we do not specified
the console/channel name.

the error message after this patch:
error: internal error: character device serial0 is not using a PTY

Signed-off-by: Luyao Huang lhu...@redhat.com
---
 src/libxl/libxl_driver.c | 4 ++--
 src/lxc/lxc_driver.c | 3 ++-
 src/qemu/qemu_driver.c   | 8 
 src/uml/uml_driver.c | 3 ++-
 src/xen/xen_driver.c | 3 ++-
 5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a7be745..c64d9be 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4292,14 +4292,14 @@ libxlDomainOpenConsole(virDomainPtr dom,
 if (!chr) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot find character device %s),
-   NULLSTR(dev_name));
+   dev_name ? dev_name : to open);
 goto cleanup;
 }
 
 if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(character device %s is not using a PTY),
-   NULLSTR(dev_name));
+   dev_name ? dev_name : NULLSTR(chr-info.alias));
 goto cleanup;
 }
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 31fb470..cc1277b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3484,7 +3484,8 @@ lxcDomainOpenConsole(virDomainPtr dom,
 
 if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(character device %s is not using a PTY), dev_name);
+   _(character device %s is not using a PTY),
+   dev_name ? dev_name : NULLSTR(chr-info.alias));
 goto cleanup;
 }
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6bb8549..282b32f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16041,14 +16041,14 @@ qemuDomainOpenConsole(virDomainPtr dom,
 if (!chr) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot find character device %s),
-   NULLSTR(dev_name));
+   dev_name ? dev_name : to open);
 goto cleanup;
 }
 
 if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(character device %s is not using a PTY),
-   NULLSTR(dev_name));
+   dev_name ? dev_name : NULLSTR(chr-info.alias));
 goto cleanup;
 }
 
@@ -16115,14 +16115,14 @@ qemuDomainOpenChannel(virDomainPtr dom,
 if (!chr) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot find channel %s),
-   NULLSTR(name));
+   name ? name : to open);
 goto cleanup;
 }
 
 if (chr-source.type != VIR_DOMAIN_CHR_TYPE_UNIX) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(channel %s is not using a UNIX socket),
-   NULLSTR(name));
+   name ? name : NULLSTR(chr-info.alias));
 goto cleanup;
 }
 
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index f09e79b..7a95458 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -2640,7 +2640,8 @@ umlDomainOpenConsole(virDomainPtr dom,
 
 if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-_(character device %s is not using a PTY), dev_name);
+_(character device %s is not using a PTY),
+   dev_name ? dev_name : NULLSTR(chr-info.alias));
 goto cleanup;
 }
 
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index da9e6f4..ce31f0f 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -2705,7 +2705,8 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom,
 
 if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(character device %s is not using a PTY), dev_name);
+   _(character device %s is not using a PTY),
+   dev_name ? dev_name : NULLSTR(chr-info.alias));
 goto cleanup;
 }
 
-- 
1.8.3.1

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


[libvirt] [PATCH] man: clarify usage of virsh blockcopy with --xml

2015-06-15 Thread Ján Tomko
The --xml option is mandatory if an XML description is used.
Otherwise the third parameter is treated as the destination.

https://bugzilla.redhat.com/show_bug.cgi?id=1206406#c3
---
 tools/virsh.pod | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9b57c8c..154922e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -989,12 +989,12 @@ unlimited. The hypervisor can choose whether to reject 
the value or
 convert it to the maximum value allowed.
 
 =item Bblockcopy Idomain Ipath { Idest [Iformat] [I--blockdev]
-| Ixml } [I--shallow] [I--reuse-external] [Ibandwidth]
+| I--xml Bfile } [I--shallow] [I--reuse-external] [Ibandwidth]
 [I--wait [I--async] [I--verbose]] [{I--pivot | I--finish}]
 [I--timeout Bseconds] [Igranularity] [Ibuf-size]
 
 Copy a disk backing image chain to a destination.  Either Idest as
-the destination file name, or Ixml as the name of an XML file containing
+the destination file name, or I--xml with the name of an XML file containing
 a top-level disk element describing the destination, must be present.
 Additionally, if Idest is given, Iformat should be specified to declare
 the format of the destination (if Iformat is omitted, then libvirt
-- 
2.3.6

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


[libvirt] [sandbox PATCH 2/4] Write /dev/vd* instead of vd* in mounts.cfg

2015-06-15 Thread Cédric Bosdonnat
Fixes a regression introduced by d74b4350: the init-qemu tool expects
/dev/vd* sources to create the block device, while we were just having
vd*. Write again /dev/vd* to mounts.cfg.
---
 libvirt-sandbox/libvirt-sandbox-builder-machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c 
b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
index 35a5816..c446447 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
@@ -303,7 +303,7 @@ static gboolean 
gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig *
 fstype = 9p;
 options = g_strdup(trans=virtio,version=9p2000.u);
 } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) {
-source = g_strdup_printf(vd%c, (char)('a' + nHostImage++));
+source = g_strdup_printf(/dev/vd%c, (char)('a' + nHostImage++));
 fstype = ext3;
 options = g_strdup();
 } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_GUEST_BIND(mconfig)) {
-- 
2.1.4

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


[libvirt] [sandbox PATCH 3/4] qemu: mount all host-images as ext4

2015-06-15 Thread Cédric Bosdonnat
To avoid troubles when mounting ext4 images, hard-code ext4 as mount
format instead of ext3.
---
 libvirt-sandbox/libvirt-sandbox-builder-machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c 
b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
index c446447..e342ba1 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
@@ -304,7 +304,7 @@ static gboolean 
gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig *
 options = g_strdup(trans=virtio,version=9p2000.u);
 } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) {
 source = g_strdup_printf(/dev/vd%c, (char)('a' + nHostImage++));
-fstype = ext3;
+fstype = ext4;
 options = g_strdup();
 } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_GUEST_BIND(mconfig)) {
 GVirSandboxConfigMountFile *mfile = 
GVIR_SANDBOX_CONFIG_MOUNT_FILE(mconfig);
-- 
2.1.4

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


[libvirt] [PATCH 1/4] build: Remove unnecessarily repeated rules for syms - def

2015-06-15 Thread Martin Kletzander
Suggested-by: Michal Prívozník mpriv...@redhat.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/Makefile.am | 20 ++--
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 0d1f58b98022..a9ebb07b1d32 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2003,29 +2003,13 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) \
chmod a-w $@-tmp  \
mv $@-tmp libvirt.syms

-libvirt.def: libvirt.syms
+%.def: %.syms
$(AM_V_GEN)rm -f -- $@-tmp $@ ; \
printf 'EXPORTS\n'  $@-tmp  \
sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d'   \
-e 's/[  ]*\(.*\)\;/\1/g' $^  $@-tmp  \
chmod a-w $@-tmp  \
-   mv $@-tmp libvirt.def
-
-libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms
-   $(AM_V_GEN)rm -f -- $@-tmp $@ ; \
-   printf 'EXPORTS\n'  $@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d'   \
-   -e 's/[  ]*\(.*\)\;/\1/g' $^  $@-tmp  \
-   chmod a-w $@-tmp  \
-   mv $@-tmp libvirt_qemu.def
-
-libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms
-   $(AM_V_GEN)rm -f -- $@-tmp $@ ; \
-   printf 'EXPORTS\n'  $@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d'   \
-   -e 's/[  ]*\(.*\)\;/\1/g' $^  $@-tmp  \
-   chmod a-w $@-tmp  \
-   mv $@-tmp libvirt_lxc.def
+   mv $@-tmp $@

 # Empty source list - it merely links a bunch of convenience libs together
 libvirt_la_SOURCES =
-- 
2.4.3

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

[libvirt] [PATCH 0/4] Various cleanups

2015-06-15 Thread Martin Kletzander
*** BLURB HERE ***

Martin Kletzander (4):
  build: Remove unnecessarily repeated rules for syms - def
  rpc: Fix possible crash when MDNSAddEntry fails
  Generate JSON with mDNS entries only when built --with-avahi
  tests: Use libvirt properly with initialization and error dispatching

 src/Makefile.am  | 20 ++--
 src/rpc/virnetserver.c   |  4 +++-
 tests/virnetservertest.c | 35 ---
 3 files changed, 25 insertions(+), 34 deletions(-)

--
2.4.3

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


Re: [libvirt] [RFC PATCHv2 2/8] threshold: expose new API in virsh

2015-06-15 Thread Peter Krempa
On Fri, Jun 12, 2015 at 13:29:26 -0600, Eric Blake wrote:
 Add a new 'virsh domblkthreshold' command to use the new API.
 
 * tools/virsh.pod (domblkthreshold): Document it.
 * tools/virsh-domain-monitor.c (cmdDomblkthreshold): New function.
 (domMonitoringCmds): Register it.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  tools/virsh-domain-monitor.c | 81 
 
  tools/virsh.pod  | 24 +
  2 files changed, 105 insertions(+)
 
 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
 index 1d4dc25..66f7571 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -571,6 +571,81 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
  }
 
  /*
 + * domblkthreshold command
 + */
 +static const vshCmdInfo info_domblkthreshold[] = {
 +{.name = help,
 + .data = N_(set domain block device write thresholds)
 +},
 +{.name = desc,
 + .data = N_(Set a threshold to get a one-shot event if block 
 +allocation exceeds that size)
 +},
 +{.name = NULL}
 +};
 +
 +static const vshCmdOptDef opts_domblkthreshold[] = {
 +{.name = domain,
 + .type = VSH_OT_DATA,
 + .flags = VSH_OFLAG_REQ,
 + .help = N_(domain name, id or uuid),
 +},
 +{.name = device,
 + .type = VSH_OT_DATA,
 + .flags = VSH_OFLAG_REQ,
 + .help = N_(block device),
 +},
 +{.name = threshold,
 + .type = VSH_OT_INT,
 + .flags = VSH_OFLAG_REQ,
 + .help = N_(new threshold, or 0 to disable),
 +},
 +{.name = percentage,

I'd rather use proportional or something else that does not hint to
parts per hundred.

 + .type = VSH_OT_BOOL,
 + .help = N_(threshold is in units of .001% instead of bytes),
 +},
 +{.name = NULL}
 +};
 +
 +static bool
 +cmdDomblkthreshold(vshControl *ctl, const vshCmd *cmd)
 +{
 +virDomainPtr dom;
 +bool ret = false;
 +const char *device = NULL;
 +unsigned long long threshold;
 +bool percentage = vshCommandOptBool(cmd, percentage);
 +unsigned int flags = 0;
 +
 +if (percentage)
 +flags |= VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE;

As well as here.

 +
 +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 +return false;
 +
 +if (vshCommandOptStringReq(ctl, cmd, device, device)  0)
 +goto cleanup;
 +if (vshCommandOptULongLong(ctl, cmd, threshold, threshold)  0)
 +goto cleanup;
 +

Peter


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

Re: [libvirt] [RFC PATCHv2 5/8] threshold: add qemu capability bit

2015-06-15 Thread Peter Krempa
On Fri, Jun 12, 2015 at 13:29:29 -0600, Eric Blake wrote:
 Track whether qemu is new enough to do block thresholds on the
 active layer.  The plan is that even if qemu is too old, the
 event handler can still be registered, but will never fire (since

Well the event handler can be registered, but the API for setting the
actual threshold value should return failure in case when qemu does not
support it.

 it is useful to bulk-install handlers); while the request to set
 a threshold will honor the capability bit and fail up front if it
 is not possible.
 
 FIXME:
 Note that qemu requires that libvirt use a node name and not a
 device name to actually use the feature.  What's more, a single
 qcow2 host resource results in two separate qemu nodes (one node
 for the guest view served by qcow2 protocol, the other node
 for the underlying host file access), so I'm working on a patch
 to qemu to automatically name all nodes (rather than having to
 hack up libvirt to supply two separate node names for a much
 more complex command line), at which point I'll need code in
 libvirt to probe the node name that got assigned by qemu.  So
 I may still need another capability bit for whether qemu is new
 enough to have the patch to auto-name all nodes.

Or perhaps just refuse to use this unless qemu is new enough to support
it?

 
 * src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCK_WRITE_THRESHOLD):
 New bit.
 * src/qemu/qemu_capabilities.c (virQEMUCapsCommands): Enable it.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  src/qemu/qemu_capabilities.c | 4 +++-
  src/qemu/qemu_capabilities.h | 3 ++-
  2 files changed, 5 insertions(+), 2 deletions(-)
 


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

Re: [libvirt] [PATCH] qemu: emulatorpin: Don't reset pinning when pinning to all cpus

2015-06-15 Thread Michal Privoznik
On 12.06.2015 15:57, Peter Krempa wrote:
 Similarly to 3813b648e9761aeed5b4f3ee7e62253a9172ce8e remove the default
 pinning assumption from emulatorpin.
 

There's no such commit. Were you referring to a commit on your local
branch perhaps?

 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1227180
 ---
  src/qemu/qemu_driver.c | 13 ++---
  1 file changed, 2 insertions(+), 11 deletions(-)
 

ACK if you update the commit message.

Michal

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


Re: [libvirt] [RFC PATCHv2 1/8] threshold: new API virDomainBlockSetWriteThreshold

2015-06-15 Thread Peter Krempa
On Fri, Jun 12, 2015 at 13:29:25 -0600, Eric Blake wrote:
 qemu 2.3 added a new QMP command block-set-write-threshold,
 which allows callers to get an interrupt when a file hits a
 write threshold, rather than the current approach of repeatedly
 polling for file allocation.  This patch prepares the API for
 callers to register to receive the event, as well as a way
 to query the threshold via virDomainListGetStats().
 
 The event is one-shot in qemu - a guest must re-register a new
 threshold after each time it triggers.  However, the
 virConnectDomainEventRegisterAny() call does not allow
 parameterization, so callers must use a pair of APIs - one
 to register the callback (one-time call) that will be used each
 time a threshold triggers for any guest disk, and another to
 repeatedly set the desired threshold (must be called each time
 a threshold should be changed).
 
 Note that the threshold can either be registered by a byte
 offset, or by a thousandth of a percentage (a value between
 0 and 10, scaled to the disk size).  But the value is
 always reported as a byte offset, even when registered as a
 percentage.  I also considered having the setup parameter be
 a double, to allow a finer resolution on percentage; with the
 choice of an integer fixed-point scale, this means a 100G
 disk can only set a threshold to a granularity of 1M, but
 that is probably sufficient for the usage.
 
 To make the patch series more digestible, this patch
 intentionally omits remote support, by using a couple of
 placeholders at a point where the compiler forces the addition
 of a case label within a switch statement.
 
 * include/libvirt/libvirt-domain.h
 (virDomainBlockSetWriteThreshold): New API.
 (virConnectDomainEventWriteThresholdCallback): New event.
 * src/libvirt_public.syms (LIBVIRT_1.2.17): Export it.
 * src/libvirt-domain.c (virDomainBlockSetWriteThreshold): New API.
 (virConnectGetAllDomainStats): New stat.
 * src/driver-hypervisor.h (virDrvDomainBlockSetWriteThreshold):
 New hypervisor entry point.
 * tools/virsh-domain.c (vshEventWriteThresholdPrint): Print new
 event.
 * tools/virsh.pod (domstats): Document new stat.
 * daemon/remote.c (domainEventCallbacks): Add stub.
 * src/conf/domain_event.c (virDomainEventDispatchDefaultFunc):
 Likewise.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  daemon/remote.c  |  2 +
  include/libvirt/libvirt-domain.h | 47 
  src/conf/domain_event.c  |  4 +-
  src/driver-hypervisor.h  |  7 +++
  src/libvirt-domain.c | 95 
 
  src/libvirt_public.syms  |  5 +++
  tools/virsh-domain.c | 23 ++
  tools/virsh.pod  |  1 +
  8 files changed, 183 insertions(+), 1 deletion(-)
 

...

 diff --git a/include/libvirt/libvirt-domain.h 
 b/include/libvirt/libvirt-domain.h
 index d851225..7514656 100644
 --- a/include/libvirt/libvirt-domain.h
 +++ b/include/libvirt/libvirt-domain.h
 @@ -1297,6 +1297,17 @@ int virDomainBlockStatsFlags 
 (virDomainPtr dom,
virTypedParameterPtr 
 params,
int *nparams,
unsigned int flags);
 +
 +typedef enum {
 +/* threshold is thousandth of a percentage (0 to 10) relative to

You managed to choose a unusual unit. Commonly used ones are 1/1000 and
1/1 000 000. Financial world also uses 1/10 000. Your unit of 1/100 000
is not among:

https://en.wikipedia.org/wiki/Parts-per_notation#Parts-per_expressions

I'd again suggest to use 1/1 000 000. Or if you want to be uber preciese
you might choose 1/(2^64 - 1).
 
 + * image size rather than byte limit */
 +VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE = (1  0),
 +} virDomainBlockSetWriteThresholdFlags;
 +int virDomainBlockSetWriteThreshold(virDomainPtr dom,
 +const char *disk,
 +unsigned long long threshold,
 +unsigned int flags);
 +
  int virDomainInterfaceStats (virDomainPtr dom,
   const char *path,
   virDomainInterfaceStatsPtr 
 stats,
 @@ -3246,6 +3257,41 @@ typedef void 
 (*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn,
   void *opaque);
 
  /**
 + * virConnectDomainEventWriteThresholdCallback:
 + * @conn: connection object
 + * @dom: domain on which the event occurred
 + * @devAlias: device alias
 + * @threshold: threshold that was exceeded, in bytes
 + * @length: length beyond @threshold that was involved in the triggering
 + *  write, or 0 if not known
 + * @opaque: application specified data
 + *
 + * The callback signature to use when registering for an event of type

[libvirt] [glib PATCH] domain config: add API to set the filesystem image format

2015-06-15 Thread Cédric Bosdonnat
Add the gvir_config_domain_filesys_set_driver_format function to allow
setting nbd driver type + image format for containers filesystems.
---
 libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 30 +++-
 libvirt-gconfig/libvirt-gconfig-domain-filesys.h |  4 
 libvirt-gconfig/libvirt-gconfig.sym  |  5 
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c 
b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
index 006a407..fffbe88 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
@@ -125,7 +125,9 @@ void 
gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys
 GVirConfigObject *node;
 
 g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys));
-node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), driver);
+if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), 
driver))) {
+node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
driver);
+}
 g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
 if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT)
 gvir_config_object_set_attribute_with_type(
@@ -137,6 +139,32 @@ void 
gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys
 g_object_unref(G_OBJECT(node));
 }
 
+void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys 
*filesys,
+  GVirConfigDomainDiskFormat 
format)
+{
+GVirConfigObject *node;
+GVirConfigDomainFilesysDriverType type = 
GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP;
+
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys));
+if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), 
driver))) {
+node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
driver);
+}
+g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
+if (format != GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW)
+type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD;
+
+gvir_config_object_set_attribute_with_type(
+node, type,
+GVIR_CONFIG_TYPE_DOMAIN_FILESYS_DRIVER_TYPE,
+type, NULL);
+
+gvir_config_object_set_attribute_with_type(
+node, format,
+GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT,
+format, NULL);
+g_object_unref(G_OBJECT(node));
+}
+
 void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys,
const char *source)
 {
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h 
b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
index 4f3973e..18c4069 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
@@ -75,6 +75,8 @@ typedef enum {
 GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT,
 GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH,
 GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE,
+GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP,
+GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD,
 } GVirConfigDomainFilesysDriverType;
 
 GType gvir_config_domain_filesys_get_type(void);
@@ -89,6 +91,8 @@ void 
gvir_config_domain_filesys_set_access_type(GVirConfigDomainFilesys *filesys
 
GVirConfigDomainFilesysAccessType type);
 void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys 
*filesys,
 
GVirConfigDomainFilesysDriverType type);
+void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys 
*filesys,
+  GVirConfigDomainDiskFormat 
format);
 void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys,
const char *source);
 void gvir_config_domain_filesys_set_ram_usage(GVirConfigDomainFilesys *filesys,
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index 407a52f..6ce1511 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -719,4 +719,9 @@ global:
gvir_config_storage_vol_target_set_compat;
 } LIBVIRT_GCONFIG_0.1.9;
 
+LIBVIRT_GCONFIG_0.2.1 {
+global:
+gvir_config_domain_filesys_set_driver_format;
+} LIBVIRT_GCONFIG_0.2.0;
+
 #  define new API here using predicted next version number 
-- 
2.1.4

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


Re: [libvirt] [PATCH] man: clarify usage of virsh blockcopy with --xml

2015-06-15 Thread Peter Krempa
On Mon, Jun 15, 2015 at 14:09:44 +0200, Ján Tomko wrote:
 The --xml option is mandatory if an XML description is used.
 Otherwise the third parameter is treated as the destination.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1206406#c3
 ---
  tools/virsh.pod | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 

ACK,

Peter


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

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

2015-06-15 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
---
 v3:
  rename qemuBuildMemoryDeviceAddr to qemuCheckMemoryDimmConflict and
  remove the refactor.

 src/qemu/qemu_command.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0a6d92f..d3f0a23 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4952,6 +4952,40 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
 }
 
 
+static bool
+qemuCheckMemoryDimmConflict(virDomainDefPtr def,
+virDomainMemoryDefPtr mem)
+{
+size_t i;
+
+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' is already being
+   used by another memory device),
+mem-info.addr.dimm.slot);
+ return true;
+ }
+
+ 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' is already being
+   used by another memory device),
+mem-info.addr.dimm.base);
+ return true;
+ }
+}
+
+return false;
+}
+
 char *
 qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
  virDomainDefPtr def,
@@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
   mem-targetNode, mem-info.alias, mem-info.alias);
 
 if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
+if (qemuCheckMemoryDimmConflict(def, mem))
+return NULL;
+
 virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot);
 virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base);
 }
-- 
1.8.3.1

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


[libvirt] [PATCH 3/4] Generate JSON with mDNS entries only when built --with-avahi

2015-06-15 Thread Martin Kletzander
One string was already used only if that condition was true, second one
is added now.  Both are used in a nicer way.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tests/virnetservertest.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/virnetservertest.c b/tests/virnetservertest.c
index 596fabedd38b..e8e91f867e45 100644
--- a/tests/virnetservertest.c
+++ b/tests/virnetservertest.c
@@ -35,6 +35,13 @@ testCreateServer(const char *host, int family)
 virNetServerClientPtr cln1 = NULL, cln2 = NULL;
 virNetSocketPtr sk1 = NULL, sk2 = NULL;
 int fdclient[2];
+const char *mdns_entry = NULL;
+const char *mdns_group = NULL;
+
+# ifdef WITH_AVAHI
+mdns_entry = libvirt-ro;
+mdns_group = libvirtTest;
+# endif

 if (socketpair(PF_UNIX, SOCK_STREAM, 0, fdclient)  0) {
 virReportSystemError(errno, %s,
@@ -44,11 +51,7 @@ testCreateServer(const char *host, int family)

 if (!(srv = virNetServerNew(10, 50, 5, 100, 10,
 120, 5, true,
-# ifdef WITH_AVAHI
-libvirtTest,
-# else
-NULL,
-# endif
+mdns_group,
 NULL,
 NULL,
 NULL,
@@ -79,9 +82,9 @@ testCreateServer(const char *host, int family)
5)))
 goto error;

-if (virNetServerAddService(srv, svc1, libvirt-ro)  0)
+if (virNetServerAddService(srv, svc1, mdns_entry)  0)
 goto error;
-if (virNetServerAddService(srv, svc2, libvirt-ro)  0)
+if (virNetServerAddService(srv, svc2, mdns_entry)  0)
 goto error;

 if (virNetSocketNewConnectSockFD(fdclient[0], sk1)  0)
-- 
2.4.3

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


[libvirt] [PATCH 2/4] rpc: Fix possible crash when MDNSAddEntry fails

2015-06-15 Thread Martin Kletzander
If virNetServerMDNSAddEntry() fails when adding a service to a server,
it doesn't decrease the number of services.  Hence access to their
members segfaults (e.g. when free()-ing the sruct).

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetserver.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 091a1dc0bc8f..2af878977a1b 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -974,8 +974,10 @@ int virNetServerAddService(virNetServerPtr srv,

 if (!virNetServerMDNSAddEntry(srv-mdnsGroup,
   mdnsEntryName,
-  port))
+  port)) {
+srv-nservices--;
 goto error;
+}
 }

 srv-services[srv-nservices-1] = svc;
-- 
2.4.3

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


[libvirt] [PATCH 4/4] tests: Use libvirt properly with initialization and error dispatching

2015-06-15 Thread Martin Kletzander
We were using complicated error printing in virnetservertest even
though we could've just dispatched the error.  Also add some good
practices that might come in handy (the code may fail without proper
initialization and event loop).

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tests/virnetservertest.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tests/virnetservertest.c b/tests/virnetservertest.c
index e8e91f867e45..d546f87764c1 100644
--- a/tests/virnetservertest.c
+++ b/tests/virnetservertest.c
@@ -119,6 +119,8 @@ testCreateServer(const char *host, int family)
 goto error;

  cleanup:
+if (!srv)
+virDispatchError(NULL);
 virObjectUnref(cln1);
 virObjectUnref(cln2);
 virObjectUnref(svc1);
@@ -235,14 +237,8 @@ static int testExecRestart(const void *opaque)
 ret = 0;

  cleanup:
-if (ret  0) {
-virErrorPtr err = virGetLastError();
-/* Rather be safe, we have lot of missing errors */
-if (err)
-fprintf(stderr, %s\n, err-message);
-else
-fprintf(stderr, %s\n, Unknown error);
-}
+if (ret  0)
+virDispatchError(NULL);
  fail:
 VIR_FREE(infile);
 VIR_FREE(outfile);
@@ -264,6 +260,12 @@ mymain(void)
 {
 int ret = 0;

+if (virInitialize()  0 ||
+virEventRegisterDefaultImpl()  0) {
+virDispatchError(NULL);
+return EXIT_FAILURE;
+}
+
 /* Hack to make it easier to generate new JSON files when
  * the RPC classes change. Just set this env var, save
  * the generated JSON, and replace the file descriptor
-- 
2.4.3

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


Re: [libvirt] [PATCH] add more domain memory functions

2015-06-15 Thread Michal Privoznik
On 12.06.2015 14:40, Vasiliy Tolstov wrote:
 * libvirt_domain_set_max_memory
 * libvirt_domain_set_memory
 * libvirt_domain_set_memory_flags
 
 Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru
 ---
  src/libvirt-php.c | 75 
 ++-
  src/libvirt-php.h |  3 +++
  2 files changed, 77 insertions(+), 1 deletion(-)

ACKed and pushed.

Michal

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


Re: [libvirt] [PATCH 0/4] Various cleanups

2015-06-15 Thread Ján Tomko
On Mon, Jun 15, 2015 at 03:08:38PM +0200, Martin Kletzander wrote:
 *** BLURB HERE ***
 
 Martin Kletzander (4):
   build: Remove unnecessarily repeated rules for syms - def
   rpc: Fix possible crash when MDNSAddEntry fails
   Generate JSON with mDNS entries only when built --with-avahi
   tests: Use libvirt properly with initialization and error dispatching
 
  src/Makefile.am  | 20 ++--
  src/rpc/virnetserver.c   |  4 +++-
  tests/virnetservertest.c | 35 ---
  3 files changed, 25 insertions(+), 34 deletions(-)

ACK series

Jan


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

Re: [libvirt] [glib PATCH] domain config: add API to set the filesystem image format

2015-06-15 Thread Cedric Bosdonnat
On Mon, 2015-06-15 at 17:12 +0200, Christophe Fergeau wrote:
 Hey,
 
 On Mon, Jun 15, 2015 at 03:37:12PM +0200, Cédric Bosdonnat wrote:
  Add the gvir_config_domain_filesys_set_driver_format function to allow
  setting nbd driver type + image format for containers filesystems.
  ---
   libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 30 
  +++-
   libvirt-gconfig/libvirt-gconfig-domain-filesys.h |  4 
   libvirt-gconfig/libvirt-gconfig.sym  |  5 
   3 files changed, 38 insertions(+), 1 deletion(-)
  
  diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c 
  b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
  index 006a407..fffbe88 100644
  --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
  +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
  @@ -125,7 +125,9 @@ void 
  gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys
   GVirConfigObject *node;
   
   g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys));
  -node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
  driver);
  +if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), 
  driver))) {
  +node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
  driver);
  +}
 
 I believe you could use gvir_config_object_replace_child() here? This
 should be split in a different commit.
 
 
   g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
   if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT)
   gvir_config_object_set_attribute_with_type(
  @@ -137,6 +139,32 @@ void 
  gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys
   g_object_unref(G_OBJECT(node));
   }
   
  +void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys 
  *filesys,
  +  
  GVirConfigDomainDiskFormat format)
  +{
  +GVirConfigObject *node;
  +GVirConfigDomainFilesysDriverType type = 
  GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP;
  +
  +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys));
  +if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), 
  driver))) {
  +node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
  driver);
  +}
 
 _replace_child() ?
 
  +g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
  +if (format != GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW)
  +type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD;
  +
  +gvir_config_object_set_attribute_with_type(
  +node, type,
  +GVIR_CONFIG_TYPE_DOMAIN_FILESYS_DRIVER_TYPE,
  +type, NULL);
  +
  +gvir_config_object_set_attribute_with_type(
  +node, format,
  +GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT,
  +format, NULL);
 
 These 2 calls can probably be grouped in a single one?
 
 I haven't looked if there are other similar situations in
 libvirt-gconfig, but silently overwriting a preexisting type attribute
 with something different when setting the format does not seem very nice
 to the library user.

What should we do in that case? Plainly return an error or just a
warning?

 
  +g_object_unref(G_OBJECT(node));
  +}
  +
   void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys 
  *filesys,
  const char *source)
   {
  diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h 
  b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
  index 4f3973e..18c4069 100644
  --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
  +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
  @@ -75,6 +75,8 @@ typedef enum {
   GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT,
   GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH,
   GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE,
  +GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP,
  +GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD,
   } GVirConfigDomainFilesysDriverType;
 
 Different commit?
 
 Could you add some small test case for the filesys node to
 tests/test/gconfig.c while you are at it?

I'll do it... but it really seems that there are a lot of untested areas
there ;)

--
Cedric

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

Re: [libvirt] [glib PATCH] domain config: add API to set the filesystem image format

2015-06-15 Thread Christophe Fergeau
On Mon, Jun 15, 2015 at 05:36:49PM +0200, Cedric Bosdonnat wrote:
 On Mon, 2015-06-15 at 17:12 +0200, Christophe Fergeau wrote:
  I haven't looked if there are other similar situations in
  libvirt-gconfig, but silently overwriting a preexisting type attribute
  with something different when setting the format does not seem very nice
  to the library user.
 
 What should we do in that case? Plainly return an error or just a
 warning?

That's a good question :) What I'm wondering is if libvirt-glib should
make these kind of checks itself, of if it should allow to build
non-sensical XML in some cases, and let libvirt errors out when it's
handed this XML. This is already the case here anyway, as even if we are
building a QEMU VM, we can call that API. So maybe don't try to be smart
in this new API entry point, and only set the format attribute when
gvir_config_domain_filesys_set_driver_format is called ?

 
  
   +g_object_unref(G_OBJECT(node));
   +}
   +
void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys 
   *filesys,
   const char *source)
{
   diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h 
   b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
   index 4f3973e..18c4069 100644
   --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
   +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
   @@ -75,6 +75,8 @@ typedef enum {
GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT,
GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH,
GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE,
   +GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP,
   +GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD,
} GVirConfigDomainFilesysDriverType;
  
  Different commit?
  
  Could you add some small test case for the filesys node to
  tests/test/gconfig.c while you are at it?
 
 I'll do it... but it really seems that there are a lot of untested areas
 there ;)

Fine with me if you don't want to go that extra mile ;)

Christophe


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

Re: [libvirt] [libvirt-php][PATCH 00/10] Couple of PHP fixes

2015-06-15 Thread Michal Privoznik
On 10.06.2015 11:29, Vasiliy Tolstov wrote:
 2015-06-10 12:23 GMT+03:00 Michal Privoznik mpriv...@redhat.com:
 Ping? No php knowledge is required to review these patches. They are
 merely Makefile and configure.ac cleanups.

 Michal
 
 
 Michal Novotny says that he busy now and you can accept/review my
 patches.. i think in this case - you can ACK ? =)
 

I went ahead and pushed this. Hopefully I did not break anything.

Michal

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


Re: [libvirt] Libvirt bite sized tasks

2015-06-15 Thread Michal Privoznik
On 15.06.2015 15:37, Dan Mossor wrote:
 On 06/15/2015 02:59 AM, Michal Privoznik wrote:
 Dear lists,

 I've just started new wiki page which aim is to summarize small and
 trivial tasks, that starting contributors can take, investigate and
 implement. The aim is to give them something easy to start with while
 not scaring them out about complexity of our code. The page can be found
 here:

 http://wiki.libvirt.org/page/BiteSizedTasks

 The name is copied from qemu wiki:

 http://wiki.qemu.org/BiteSizedTasks

 Please, feel free to extend the list. I plan to use it in the future
 when interviewing GSoC candidates.

 Michal

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

 
 According to Laine [0], self-registration for the wiki is disabled. What
 would be the correct avenue of approach for those of us that are willing
 to contribute but don't have libvirt wiki accounts? Is there work in
 progress to tie it to FAS or OpenID?

You mean you want to extend the list of task or do you want to work on
an item and get your patches merged? The former, I guess, it's question
for DV. But I can look into that too. The latter - there's no need
register on the wiki. Just work on the patches and probably post URL in
the commit message so that we can remove the items from the list.

Michal

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


Re: [libvirt] [PATCH] qemu: emulatorpin: Don't reset pinning when pinning to all cpus

2015-06-15 Thread Peter Krempa
On Mon, Jun 15, 2015 at 16:04:29 +0200, Michal Privoznik wrote:
 On 12.06.2015 15:57, Peter Krempa wrote:
  Similarly to 3813b648e9761aeed5b4f3ee7e62253a9172ce8e remove the default
  pinning assumption from emulatorpin.
  
 
 There's no such commit. Were you referring to a commit on your local
 branch perhaps?

Indeed, the upstream commit ID for the one I wanted to refer to is

a02a161bb8a6caf0db4dd446ed1cdf53d97b40b9

 
  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1227180
  ---
   src/qemu/qemu_driver.c | 13 ++---
   1 file changed, 2 insertions(+), 11 deletions(-)
  
 
 ACK if you update the commit message.

I'll fix it and push. Thanks.



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

Re: [libvirt] [glib PATCH] domain config: add API to set the filesystem image format

2015-06-15 Thread Christophe Fergeau
Hey,

On Mon, Jun 15, 2015 at 03:37:12PM +0200, Cédric Bosdonnat wrote:
 Add the gvir_config_domain_filesys_set_driver_format function to allow
 setting nbd driver type + image format for containers filesystems.
 ---
  libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 30 
 +++-
  libvirt-gconfig/libvirt-gconfig-domain-filesys.h |  4 
  libvirt-gconfig/libvirt-gconfig.sym  |  5 
  3 files changed, 38 insertions(+), 1 deletion(-)
 
 diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c 
 b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
 index 006a407..fffbe88 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
 +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
 @@ -125,7 +125,9 @@ void 
 gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys
  GVirConfigObject *node;
  
  g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys));
 -node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
 driver);
 +if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), 
 driver))) {
 +node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
 driver);
 +}

I believe you could use gvir_config_object_replace_child() here? This
should be split in a different commit.


  g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
  if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT)
  gvir_config_object_set_attribute_with_type(
 @@ -137,6 +139,32 @@ void 
 gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys
  g_object_unref(G_OBJECT(node));
  }
  
 +void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys 
 *filesys,
 +  GVirConfigDomainDiskFormat 
 format)
 +{
 +GVirConfigObject *node;
 +GVirConfigDomainFilesysDriverType type = 
 GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP;
 +
 +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys));
 +if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), 
 driver))) {
 +node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), 
 driver);
 +}

_replace_child() ?

 +g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node));
 +if (format != GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW)
 +type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD;
 +
 +gvir_config_object_set_attribute_with_type(
 +node, type,
 +GVIR_CONFIG_TYPE_DOMAIN_FILESYS_DRIVER_TYPE,
 +type, NULL);
 +
 +gvir_config_object_set_attribute_with_type(
 +node, format,
 +GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT,
 +format, NULL);

These 2 calls can probably be grouped in a single one?

I haven't looked if there are other similar situations in
libvirt-gconfig, but silently overwriting a preexisting type attribute
with something different when setting the format does not seem very nice
to the library user.


 +g_object_unref(G_OBJECT(node));
 +}
 +
  void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys,
 const char *source)
  {
 diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h 
 b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
 index 4f3973e..18c4069 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
 +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
 @@ -75,6 +75,8 @@ typedef enum {
  GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT,
  GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH,
  GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE,
 +GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP,
 +GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD,
  } GVirConfigDomainFilesysDriverType;

Different commit?

Could you add some small test case for the filesys node to
tests/test/gconfig.c while you are at it?

Christophe


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

Re: [libvirt] [PATCH v2 1/1] qemu: monitor: Add memory balloon support for virtio-ccw

2015-06-15 Thread Ján Tomko
On Wed, Jun 10, 2015 at 09:02:36AM +0200, Boris Fiuczynski wrote:
 The search for the memory balloon driver object is extended by a
 second known name virtio-balloon-ccw in support for virtio-ccw.
 
 Signed-off-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com
 Reviewed-by: Daniel Hansel daniel.han...@linux.vnet.ibm.com
 Reviewed-by: Eric Farman far...@linux.vnet.ibm.com
 Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com
 ---
  src/qemu/qemu_monitor.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 33600f0..6f2f4a9 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -1069,9 +1069,9 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
 virJSONValuePtr options)
  
  

ACK and pushed.

Jan


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

Re: [libvirt] [PATCH] Fix virsh edit and virt-xml-validate with SCSI LUN

2015-06-15 Thread Eric Farman



On 06/15/2015 05:03 PM, John Ferlan wrote:

Something I didn't catch on pass1, but perhaps an important distinction

title:

Resolve issues with SCSI LUN hostdev device addressing


Sounds good!  I'll work that into the split patches.




Coincidentally - there's a bz regarding the address setting between
SCSI disk and hostdev which is assigned to me and was near the top
of my todo list... https://bugzilla.redhat.com/show_bug.cgi?id=1210587
so this has helped put me in the right frame of reference!

...


Need to add some XML in order to test the longer values - eg, xml2xml
and/or xml2args in order to validate that what you read in is what gets
printed out and that what you read in gets sent to the hypervisor
correctly.

I wrestled with this part, because the xml2args tests pass the SCSI
[host:bus:target:unit] address to testSCSIDeviceGetSgName, which ignores
all inputs and returns /dev/sg0.  The reverse had a similar gotcha.
I'll dig at xml2xml, and see what I can come up with.


me wonders what qemu dos with a 20 digit unit number...

So speaking of qemu... If you look at qemuBuildDriveStr you will see how
the device address is passed down to qemu... That function needs some
adjustment too it seems.

This function doesn't appear to be invoked for an address tag within a
hostdev element, either as part of the source subelements or as the
address being created for the guest.  Rather, it seems to turn up as
part of a disk element, which (unless I'm mistaken) relies on the host
/dev address instead of [host:bus:target:unit].

Ah... so back to my title comment ;-) - hostdev...  That would mean
qemuBuildSCSIHostdevDevStr, which does have a printing of bus, target,
unit.  Still wondering about how qemu would handle it (haven't got that
far yet), but that seems to be what you point out in the next paragraph.


Speaking of the address that is created in the guest, though, I left
that as-is because virtio defines LUN addresses up to 16,384 and is
enforced in KVM/QEMU.  Since I can't create a unit address in the guest
larger than that, leaving that defined as an int is okay.


And so regardless of what you did for libvirt, kvm/qemu wouldn't be able
to handle it?  Perhaps then that needs to be checked somehow... Is this
something qemu/kvm needs to support?


I'll defer to you, but kvm/qemu enforces things okay and libvirt fails 
gracefully.  Example:


# cat disk.xml
hostdev mode='subsystem' type='scsi'
  source
adapter name='scsi_host0'/
address bus='0' target='14' unit='1074872354'/
  /source
  address type='drive' controller='0' bus='0' target='0' 
unit='16384'/

/hostdev
# virsh attach-device lmb_guest disk.xml
error: Failed to attach device from disk.xml
error: internal error: unable to execute QEMU command 'device_add': bad 
scsi device lun: 16384


# vim disk.xml
# cat disk.xml
hostdev mode='subsystem' type='scsi'
  source
adapter name='scsi_host0'/
address bus='0' target='14' unit='1074872354'/
  /source
  address type='drive' controller='0' bus='0' target='0' 
unit='16383'/

/hostdev
# virsh attach-device lmb_guest disk.xml
Device attached successfully

This looks reasonable to me because virtio only permits 256 targets, and 
16,384 units per targets.  So there's no concern with the corresponding 
int fields being overrun.



Again I haven't looked at those
sources yet.  Is there a specific hypervisor that isn't working because
libvirt isn't passing the correct address value?  This is the what is
the bug you're trying to fix type question...


I'm using qemu/kvm, but I haven't seen anything besides the virsh edit 
and virt-xml-validate errors when the hostdev-source-address-unit 
value is larger than two digits.  But this really boils to your later 
comment about the patch being split, to explain the sequence of events 
that brought me here.  I have it broken into the patches you had 
suggested, plus two others (one doc, one xml schema which is what 
started this), but haven't tested them independently yet.  Tomorrow, I hope.


Eric




I didn't dig into the qemu sources yet, but


Looks like I also lost my train of thought here ;-)


diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1781996..e7a8e1a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2827,7 +2827,7 @@
 ddDrive addresses have the following additional
   attributes: codecontroller/code (a 2-digit controller
   number), codebus/code (a 2-digit bus number),
-codetarget/code (a 2-digit bus number),
+codetarget/code (a 2-digit target number),
   and codeunit/code (a 2-digit unit number on the bus).
 /dd
 dtcodetype='virtio-serial'/code/dt

Interesting there's no 'scsi' in here (of course you're removing it
below, but that leaves the address as unknown

Is it?  See below, but I thought that got hardcoded somewhere because
it's in a hostdev.  I'll doublecheck.


[libvirt] [PATCH libvirt-python] virPyDictToTypedParams: packing lists of values

2015-06-15 Thread Pavel Boldin
Pack a list or a tuple of values passed to a Python method to the
multi-value parameter.
---
 libvirt-override.c | 228 ++---
 1 file changed, 129 insertions(+), 99 deletions(-)

diff --git a/libvirt-override.c b/libvirt-override.c
index 588dac1..45c8afc 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -278,6 +278,126 @@ typedef virPyTypedParamsHint *virPyTypedParamsHintPtr;
 #  define libvirt_PyString_Check PyString_Check
 # endif
 
+static int
+virPyDictToTypedParamOne(virTypedParameterPtr *params,
+ int *n,
+ int *max,
+ virPyTypedParamsHintPtr hints,
+ int nhints,
+ const char *keystr,
+ PyObject *value)
+{
+int rv = -1, type = -1;
+size_t i;
+
+for (i = 0; i  nhints; i++) {
+if (STREQ(hints[i].name, keystr)) {
+type = hints[i].type;
+break;
+}
+}
+
+if (type == -1) {
+if (libvirt_PyString_Check(value)) {
+type = VIR_TYPED_PARAM_STRING;
+} else if (PyBool_Check(value)) {
+type = VIR_TYPED_PARAM_BOOLEAN;
+} else if (PyLong_Check(value)) {
+unsigned long long ull = PyLong_AsUnsignedLongLong(value);
+if (ull == (unsigned long long) -1  PyErr_Occurred())
+type = VIR_TYPED_PARAM_LLONG;
+else
+type = VIR_TYPED_PARAM_ULLONG;
+#if PY_MAJOR_VERSION  3
+} else if (PyInt_Check(value)) {
+if (PyInt_AS_LONG(value)  0)
+type = VIR_TYPED_PARAM_LLONG;
+else
+type = VIR_TYPED_PARAM_ULLONG;
+#endif
+} else if (PyFloat_Check(value)) {
+type = VIR_TYPED_PARAM_DOUBLE;
+}
+}
+
+if (type == -1) {
+PyErr_Format(PyExc_TypeError,
+ Unknown type of \%s\ field, keystr);
+goto cleanup;
+}
+
+switch ((virTypedParameterType) type) {
+case VIR_TYPED_PARAM_INT:
+{
+int val;
+if (libvirt_intUnwrap(value, val)  0 ||
+virTypedParamsAddInt(params, n, max, keystr, val)  0)
+goto cleanup;
+break;
+}
+case VIR_TYPED_PARAM_UINT:
+{
+unsigned int val;
+if (libvirt_uintUnwrap(value, val)  0 ||
+virTypedParamsAddUInt(params, n, max, keystr, val)  0)
+goto cleanup;
+break;
+}
+case VIR_TYPED_PARAM_LLONG:
+{
+long long val;
+if (libvirt_longlongUnwrap(value, val)  0 ||
+virTypedParamsAddLLong(params, n, max, keystr, val)  0)
+goto cleanup;
+break;
+}
+case VIR_TYPED_PARAM_ULLONG:
+{
+unsigned long long val;
+if (libvirt_ulonglongUnwrap(value, val)  0 ||
+virTypedParamsAddULLong(params, n, max, keystr, val)  0)
+goto cleanup;
+break;
+}
+case VIR_TYPED_PARAM_DOUBLE:
+{
+double val;
+if (libvirt_doubleUnwrap(value, val)  0 ||
+virTypedParamsAddDouble(params, n, max, keystr, val)  0)
+goto cleanup;
+break;
+}
+case VIR_TYPED_PARAM_BOOLEAN:
+{
+bool val;
+if (libvirt_boolUnwrap(value, val)  0 ||
+virTypedParamsAddBoolean(params, n, max, keystr, val)  0)
+goto cleanup;
+break;
+}
+case VIR_TYPED_PARAM_STRING:
+{
+char *val;;
+if (libvirt_charPtrUnwrap(value, val)  0 ||
+!val ||
+virTypedParamsAddString(params, n, max, keystr, val)  0) {
+VIR_FREE(val);
+goto cleanup;
+}
+VIR_FREE(val);
+break;
+}
+case VIR_TYPED_PARAM_LAST:
+break; /* unreachable */
+}
+
+rv = 0;
+
+ cleanup:
+return rv;
+}
+
+
 /* Automatically convert dict into type parameters based on types reported
  * by python. All integer types are converted into LLONG (in case of a negative
  * value) or ULLONG (in case of a positive value). If you need different
@@ -300,7 +420,6 @@ virPyDictToTypedParams(PyObject *dict,
 Py_ssize_t pos = 0;
 #endif
 virTypedParameterPtr params = NULL;
-size_t i;
 int n = 0;
 int max = 0;
 int ret = -1;
@@ -313,112 +432,23 @@ virPyDictToTypedParams(PyObject *dict,
 return -1;
 
 while (PyDict_Next(dict, pos, key, value)) {
-int type = -1;
-
 if (libvirt_charPtrUnwrap(key, keystr)  0 ||
 !keystr)
 goto cleanup;
 
-for (i = 0; i  nhints; i++) {
-if (STREQ(hints[i].name, keystr)) {
-type = hints[i].type;
-break;
-}
-}
+if (PyList_Check(value) || PyTuple_Check(value)) {
+Py_ssize_t i, size = PySequence_Size(value);
 
-if (type == -1) {
-if (libvirt_PyString_Check(value)) {
-

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

2015-06-15 Thread lhuang


On 06/16/2015 08:27 AM, zhang bo wrote:

On 2015/6/15 20:33, Luyao Huang wrote:


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

+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' is already being
+   used by another memory device),
+mem-info.addr.dimm.slot);
+ return true;
+ }
+
+ if (mem-info.addr.dimm.base != 0  tmp-info.addr.dimm.base != 0 
+ mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {


Equal the memory device base with 0 means: make sure the 2 memory device 
base is specified (not let qemu auto assign the base).


So the logic here is :
1. make sure memory device A and B base is not auto assign mode.
2. then equal the base address


The logic here equals:
  if (mem-info.addr.dimm.base != 0 
  mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {
will it be better like this?



Indeed, your code looks better.

Thanks a lot for your review.


+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(memory device base '0x%llx' is already being
+   used by another memory device),
+mem-info.addr.dimm.base);
+ return true;
+ }
+}
+
+return false;
+}
+
  char *
  qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
   virDomainDefPtr def,
@@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
mem-targetNode, mem-info.alias, mem-info.alias);
  
  if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {

+if (qemuCheckMemoryDimmConflict(def, mem))
+return NULL;
+
  virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot);
  virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base);
  }





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


Re: [libvirt] [RFC PATCHv2 5/8] threshold: add qemu capability bit

2015-06-15 Thread Eric Blake
On 06/15/2015 07:33 AM, Peter Krempa wrote:
 On Fri, Jun 12, 2015 at 13:29:29 -0600, Eric Blake wrote:
 Track whether qemu is new enough to do block thresholds on the
 active layer.  The plan is that even if qemu is too old, the
 event handler can still be registered, but will never fire (since
 
 Well the event handler can be registered, but the API for setting the
 actual threshold value should return failure in case when qemu does not
 support it.
 
 it is useful to bulk-install handlers); while the request to set
 a threshold will honor the capability bit and fail up front if it
 is not possible.

Yes, that's what the second half of the sentence was trying to say. I
can reorder things to make it more obvious:

If qemu is too old, attempts to register a threshold value will return
failure; but the event handler callback can still be registered
successfully (and will just never fire).


 FIXME:
 Note that qemu requires that libvirt use a node name and not a
 device name to actually use the feature.  What's more, a single
 qcow2 host resource results in two separate qemu nodes (one node
 for the guest view served by qcow2 protocol, the other node
 for the underlying host file access), so I'm working on a patch
 to qemu to automatically name all nodes (rather than having to
 hack up libvirt to supply two separate node names for a much
 more complex command line), at which point I'll need code in
 libvirt to probe the node name that got assigned by qemu.  So
 I may still need another capability bit for whether qemu is new
 enough to have the patch to auto-name all nodes.
 
 Or perhaps just refuse to use this unless qemu is new enough to support
 it?

Yeah, after the weekend, my current work-in-progress is to patch qemu to
auto-name nodes, and to make libvirt refuse to allow threshold settings
unless auto-named nodes are detected in addition to the write-threshold
command, since the qemu auto-name patch can be fairly easy backported by
distros.  It's thus turning into two capability bits, both of which must
be present to use thresholds (but where the callback can be registered
even if either or both capabilities are missing, just that the callback
will never fire in those cases).

-- 
Eric Blake   eblake 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/2] Adjust netfs CIFS/Samba formatting

2015-06-15 Thread John Ferlan


On 06/03/2015 01:06 PM, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1186969
 
 The first patch follows a previous change to fix/adjust the gluster
 specific rng formatting
 
 The second patch resolves the particular issue ensuring to generate the
 source path with the //%s/%s instead of %s:%s and with the -o group
 to connect to the Samba serbver.
 
 John Ferlan (2):
   storage: Fix the schema and add tests for cifs pool
   storage: Generate correct parameters for CIFS
 
  docs/formatstorage.html.in  |  7 --
  docs/schemas/storagepool.rng| 24 +++-
  docs/storage.html.in|  3 ++-
  src/storage/storage_backend_fs.c| 29 
 -
  tests/storagepoolxml2xmlin/pool-netfs-cifs.xml  | 12 ++
  tests/storagepoolxml2xmlout/pool-netfs-cifs.xml | 15 +
  tests/storagepoolxml2xmltest.c  |  1 +
  7 files changed, 82 insertions(+), 9 deletions(-)
  create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-cifs.xml
  create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-cifs.xml
 

Ping... I know... Samba/Windows... no one's complained it's broken from
day 1...

Thanks -

John

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


Re: [libvirt] [PATCH 0/2] Fix a couple issues found with disk backend

2015-06-15 Thread John Ferlan


On 06/08/2015 08:25 AM, John Ferlan wrote:
 Validation of https://bugzilla.redhat.com/show_bug.cgi?id=1181062 raised
 a couple of questions about XML format and the defaults - these patches
 address those questions.
 
 John Ferlan (2):
   docs: Adjust Disk storage rng
   storage: Force setting of disk format type
 
  docs/schemas/storagepool.rng   | 2 +-
  src/storage/storage_backend_disk.c | 5 +++--
  2 files changed, 4 insertions(+), 3 deletions(-)
 

ping

Thanks -

John

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


Re: [libvirt] [PATCH v2] storage: Need to set secrettype for direct iscsi disk volume

2015-06-15 Thread John Ferlan

...

 +/* Shared processing code with storage pools can leave
 + * this empty, but disk formatting uses it as does command
 + * creation - so use the secretType to attempt to fill it 
 in.
 + */
 +if (!authdef-secrettype) {
 +const char *secrettype =
 +virSecretUsageTypeToString(authdef-secretType);
 +if (VIR_STRDUP(authdef-secrettype, secrettype)  0)
 +goto error;
 +}

 So _virStorageAuthDef has both @secrettype and @secretType? That's
 rather perplexing.

 
 Yeah - I think it was a convenience perhaps when trying to combine the
 various authdef functions that had existed.  I look to generate a
 followup that removes the need for it
 

Ahh - now that a few more caffeine cells have been coursing through my
bloodstream - I remember what the difference is and I also see I made a
mistake in which I was using (sigh).

The 'secrettype' is what is read from XML secret type='%s' It is
optional and is used differently depending whether it's in the domain
'disk' or the storage pool 'source' definition.

For the domain disk, there is no auth 'type' field, while there is a
secret 'type' field.

For the storage pool, there is an auth 'type' field, while there is no
secret 'type' field.

The 'secretType' is used to determine whether a 'usage' or 'uuid' was
found the secret... XML and virSecretUsageTypeToString does the magic
to properly format. So using it to format the secrettype is wrong.

Of course because I added the VIR_STORAGE_TYPE_VOLUME check to ignore
auth_secret_usage and eventually virStorageTranslateDiskSourcePool is
run, so I didn't see my mistake, but now I do.

I'll post an adjustment - the current patch works because eventually
virStorageTranslateDiskSourcePool overwrites the mistake.


John

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


Re: [libvirt] [PATCH] Fix virsh edit and virt-xml-validate with SCSI LUN

2015-06-15 Thread Eric Farman



On 06/12/2015 04:49 PM, John Ferlan wrote:


On 06/10/2015 11:22 AM, Eric Farman wrote:

Defining a domain with a SCSI disk attached via a hostdev
tag and a source address unit value longer than two digits
causes an error when editing the domain with virsh edit,
even if no changes are made to the domain definition.
The error suggests invalid XML, somewhere:

   # virsh edit lmb_guest
   error: XML document failed to validate against schema:
   Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
   Extra element devices in interleave
   Element domain failed to validate content

The virt-xml-validate tool fails with a similar error:

   # virt-xml-validate lmb_guest.xml
   Relax-NG validity error : Extra element devices in interleave
   lmb_guest.xml:17: element devices: Relax-NG validity error :
   Element domain failed to validate content
   lmb_guest.xml fails to validate

The hostdev tag requires a source address to be specified,
which includes bus, target, and unit address attributes.
According to the SCSI Architecture Model spec (section
4.9 of SAM-2), a LUN address is 64 bits and thus could be
up to 20 decimal digits long.  Unfortunately, the XML
schema limits this string to just two digits.  Similarly,
the target field can be up to 32 bits in length, which
would be 10 decimal digits.

   # lsscsi -xx
   [0:0:19:0x40224011]  diskIBM  2107900  3.44 /dev/sda
   # lsscsi
   [0:0:19:1074872354]diskIBM  2107900  3.44  /dev/sda
   # cat lmb_guest.xml
   domain type='kvm'
 namelmb_guest/name
 memory unit='MiB'1024/memory
   ...trimmed...
 devices
   controller type='scsi' model='virtio-scsi' index='0'/
   hostdev mode='subsystem' type='scsi'
 source
   adapter name='scsi_host0'/
   address bus='0' target='19' unit='1074872354'/
 /source
   /hostdev
   ...trimmed...

Since the reference unit and target fields are used in
several places in the XML schema, create a separate one
specific for SCSI Logical Units that will permit the
greater length.  Also, expand the definition of the SCSI
unit field from an int to a long long, to cover the
possible size. This permits both the validation utility
and the virsh edit command to succeed when a hostdev
tag is included.


Nice explanation - helped set the stage quite well!

Of course you've just opened Pandora's box of questions...


Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
Reviewed-by: Matthew Rosato mjros...@linux.vnet.ibm.com
Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com
Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com
---
  docs/formatdomain.html.in | 10 +++---
  docs/schemas/domaincommon.rng | 14 --
  src/conf/domain_audit.c   |  2 +-
  src/conf/domain_conf.c|  4 ++--
  src/conf/domain_conf.h|  2 +-
  src/qemu/qemu_command.h   |  2 +-
  src/qemu/qemu_hotplug.c   |  4 ++--
  src/util/virhostdev.c |  6 +++---
  src/util/virscsi.c| 16 
  src/util/virscsi.h|  8 
  tests/testutilsqemu.c |  2 +-
  tools/virsh-domain.c  |  6 +++---
  12 files changed, 45 insertions(+), 31 deletions(-)


Need to add some XML in order to test the longer values - eg, xml2xml
and/or xml2args in order to validate that what you read in is what gets
printed out and that what you read in gets sent to the hypervisor
correctly.


I wrestled with this part, because the xml2args tests pass the SCSI 
[host:bus:target:unit] address to testSCSIDeviceGetSgName, which ignores 
all inputs and returns /dev/sg0.  The reverse had a similar gotcha.  
I'll dig at xml2xml, and see what I can come up with.



   me wonders what qemu dos with a 20 digit unit number...

So speaking of qemu... If you look at qemuBuildDriveStr you will see how
the device address is passed down to qemu... That function needs some
adjustment too it seems.


This function doesn't appear to be invoked for an address tag within a 
hostdev element, either as part of the source subelements or as the 
address being created for the guest.  Rather, it seems to turn up as 
part of a disk element, which (unless I'm mistaken) relies on the host 
/dev address instead of [host:bus:target:unit].


Speaking of the address that is created in the guest, though, I left 
that as-is because virtio defines LUN addresses up to 16,384 and is 
enforced in KVM/QEMU.  Since I can't create a unit address in the guest 
larger than that, leaving that defined as an int is okay.




I didn't dig into the qemu sources yet, but


diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1781996..e7a8e1a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2827,7 +2827,7 @@
ddDrive addresses have the following additional
  attributes: codecontroller/code (a 2-digit controller
  number), codebus/code (a 2-digit bus number),
-

[libvirt] Publishing a Python libvirt Howto/Reference Guide

2015-06-15 Thread David Ashley

All -

I represent the Fedora Docs Team. We are interested in publishing a 
Python libvirt HowTo/Reference Guide for users. We feel that there is a 
gap with using virtualization from a scripting environment. While virsh 
fills this gap for simple scripting purposes, a more powerful scripting 
language is needed for deep administrative purposes. Python is the 
obvious choice and the libvirt team has provided a great Python module. 
What is really missing are examples of using the module in both simple 
and deeper tasks i.e. something more than just how to start and stop a 
domain.


We propose to create such a document. While we can use pydoc to create 
the reference material, we need some really good examples that can teach 
a user how to get real work done using the classes. We also propose to 
share the document with the libvirt team taking either an upstream or 
downstream role in maintaining the document. The document would be 
written in DocBook and published using the Publican system. This would 
allow you to create your own brand for the document (look and feel) 
while maintaining compatible source for both our teams. It would also 
allow the source to be maintained by both teams.


If this sounds interesting to you we would like to begin our planning 
for the document in the near future with one or more representatives 
from the libvirt team on board. Please let either myself or Laura Novich 
(cc'ed on this email) know if this proposal meets with your approval.


W. David Ashley
w.david.ash...@gmail.com
Fedora Docs Team


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


Re: [libvirt] [RFC PATCHv2 2/8] threshold: expose new API in virsh

2015-06-15 Thread Eric Blake
On 06/15/2015 07:29 AM, Peter Krempa wrote:
 On Fri, Jun 12, 2015 at 13:29:26 -0600, Eric Blake wrote:
 Add a new 'virsh domblkthreshold' command to use the new API.

 * tools/virsh.pod (domblkthreshold): Document it.
 * tools/virsh-domain-monitor.c (cmdDomblkthreshold): New function.
 (domMonitoringCmds): Register it.

 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  tools/virsh-domain-monitor.c | 81 
 
  tools/virsh.pod  | 24 +
  2 files changed, 105 insertions(+)


 +{.name = percentage,
 
 I'd rather use proportional or something else that does not hint to
 parts per hundred.

Good idea.  It also means I need to tweak the flag name in patch 1.

I also wonder if I should teach virsh to allow '--percentage 72.5'
(meaning 72.5%) in addition to '--proportion 725000' (if we switch to
parts-per-million)?  Floating point parameter parsing in virsh could be
interesting...


 +if (percentage)
 +flags |= VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE;
 
 As well as here.

Ah, so you caught the same naming conundrum as I mentioned above :)

-- 
Eric Blake   eblake 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] configure: Remove check for pkcheck_supports_uid

2015-06-15 Thread Guido Günther
On Mon, Jun 15, 2015 at 11:33:58AM +0200, Martin Kletzander wrote:
 On Fri, Jun 05, 2015 at 01:01:34PM +0200, Guido Günther wrote:
 We're using Polkit's DBus API so no need to check wether this feature is
 supported. We don't use the result or the path to the pkcheck program
 anywhere.
 ---
 configure.ac | 9 -
 1 file changed, 9 deletions(-)
 
 
 ACK, this is not needed anymore.

Pushed. Thanks.
 -- Guido

 
 diff --git a/configure.ac b/configure.ac
 index abf4436..073624b 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1362,15 +1362,6 @@ if test x$with_polkit = xyes || test 
 x$with_polkit = xcheck; then
   dnl Check for new polkit first - just a binary
   AC_PATH_PROG([PKCHECK_PATH],[pkcheck], [], [/usr/sbin:$PATH])
   if test x$PKCHECK_PATH != x ; then
 -AC_DEFINE_UNQUOTED([PKCHECK_PATH],[$PKCHECK_PATH],[Location of 
 pkcheck program])
 -AC_MSG_CHECKING([whether pkcheck supports uid value])
 -pkcheck_supports_uid=`$PKG_CONFIG --variable pkcheck_supports_uid 
 polkit-gobject-1`
 -if test x$pkcheck_supports_uid = xtrue; then
 -  AC_MSG_RESULT([yes])
 -  AC_DEFINE_UNQUOTED([PKCHECK_SUPPORTS_UID], 1, [Pass uid to pkcheck])
 -else
 -  AC_MSG_RESULT([no])
 -fi
 AC_DEFINE_UNQUOTED([WITH_POLKIT], 1,
 [use PolicyKit for UNIX socket access checks])
 AC_DEFINE_UNQUOTED([WITH_POLKIT1], 1,
 --
 2.1.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


Re: [libvirt] [PATCH/RFC] Add missing delta from Ubuntu to apparmor profiles

2015-06-15 Thread intrigeri
Hi,

Stefan Bader wrote (20 May 2015 10:11:45 GMT) :
 Hm was there not something which I was waiting for feedback
 from you?

My (possibly incomplete) records say that I've tested the latest
proposed patch set back in February (85iof8v6j5@boum.org).

 Since I lost most context by now, I will try to find my most recent proposal
 again and try to get it moved into present state of packages.

Thanks!

Cheers,
-- 
intrigeri

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


Re: [libvirt] [RFC PATCHv2 1/8] threshold: new API virDomainBlockSetWriteThreshold

2015-06-15 Thread Eric Blake
On 06/15/2015 07:19 AM, Peter Krempa wrote:
 On Fri, Jun 12, 2015 at 13:29:25 -0600, Eric Blake wrote:
 qemu 2.3 added a new QMP command block-set-write-threshold,
 which allows callers to get an interrupt when a file hits a
 write threshold, rather than the current approach of repeatedly
 polling for file allocation.  This patch prepares the API for
 callers to register to receive the event, as well as a way
 to query the threshold via virDomainListGetStats().


 +
 +typedef enum {
 +/* threshold is thousandth of a percentage (0 to 10) relative to
 
 You managed to choose a unusual unit. Commonly used ones are 1/1000 and
 1/1 000 000. Financial world also uses 1/10 000. Your unit of 1/100 000
 is not among:
 
 https://en.wikipedia.org/wiki/Parts-per_notation#Parts-per_expressions
 
 I'd again suggest to use 1/1 000 000. Or if you want to be uber preciese
 you might choose 1/(2^64 - 1).

Francesco, what precision would you like?  Parts per million seems okay
to me, if we want an order of magnitude closer; and I don't think we
need anything beyond that.  Or if parts per thousand is sufficient, that
leads to smaller numbers on input.  But it's pretty trivial for me to
adjust the code to a different base, for whatever people would like.


 + * The contents of @devAlias will be vda when the threshold is triggered
 + * on the active layer of guest disk vda.  Some hypervisors also support
 + * threshold reporting on backing images, such as during a block commit;
 + * when that happens, @devAlias will be vda[1] for the backingStore at
 + * index 1 within the chain of host resources for guest disk vda.
 
 Is it perhaps worth to include a optional field that will contain the
 file path since most use cases of this event will use a local block
 device with the event? iscsi and NBD block devices then could return the
 field empty as we now do in the bulk stats API

Good idea - since the input accepts local path names instead of vda,
the output can display both where a local name is available.  I'll
update my code.


 + *
 + * The @disk parameter is either the device target shorthand (the
 + * target dev='...'/ sub-element, such as vda), or (since 0.9.8)
 
 Since this will be added in 1.3.0 (or 1.2.15) the since statement is
 not exactly true.

Copy-and-paste strikes again :)

-- 
Eric Blake   eblake 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] conf: Fix virDomainObjGetDefs when getting persistent config on a live vm

2015-06-15 Thread Peter Krempa
On Fri, Jun 12, 2015 at 14:41:03 +0200, Peter Krempa wrote:
 If @flags contains only VIR_DOMAIN_AFFECT_CONFIG and @vm is active, the
 function would return the active config rather than the persistent one
 that it should return. This happened due to the fact that
 virDomainObjGetDefs was checking the updated flags which may not contain
 VIR_DOMAIN_AFFECT_LIVE if it is not requested even if @vm is active.
 
 The mistake was caught by the virt-test suite.
 ---
  src/conf/domain_conf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 2e79610..fd38c5d 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -2933,7 +2933,7 @@ virDomainObjGetDefs(virDomainObjPtr vm,
  if (virDomainObjUpdateModificationImpact(vm, flags)  0)
  return -1;
 
 -if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 +if (virDomainObjIsActive(vm)) {
  if (liveDef)
  *liveDef = vm-def;

Self NACK, this patch is incomplete and would not work correctly.

Peter


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

Re: [libvirt] Libvirt bite sized tasks

2015-06-15 Thread Dan Mossor

On 06/15/2015 10:11 AM, Michal Privoznik wrote:

On 15.06.2015 15:37, Dan Mossor wrote:

On 06/15/2015 02:59 AM, Michal Privoznik wrote:

Dear lists,

I've just started new wiki page which aim is to summarize small and
trivial tasks, that starting contributors can take, investigate and
implement. The aim is to give them something easy to start with while
not scaring them out about complexity of our code. The page can be found
here:

http://wiki.libvirt.org/page/BiteSizedTasks

The name is copied from qemu wiki:

http://wiki.qemu.org/BiteSizedTasks

Please, feel free to extend the list. I plan to use it in the future
when interviewing GSoC candidates.

Michal

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



According to Laine [0], self-registration for the wiki is disabled. What
would be the correct avenue of approach for those of us that are willing
to contribute but don't have libvirt wiki accounts? Is there work in
progress to tie it to FAS or OpenID?


You mean you want to extend the list of task or do you want to work on
an item and get your patches merged? The former, I guess, it's question
for DV. But I can look into that too. The latter - there's no need
register on the wiki. Just work on the patches and probably post URL in
the commit message so that we can remove the items from the list.

Michal

Oh, I misread the original email. I thought this was for short how-to 
contributions to the wiki for users, not code contributions. I'm not a 
developer, but a user and teacher. I'd be willing to write wiki pages, 
but I don't do code.


Dan

--
Dan Mossor, RHCSA
Systems Engineer
Fedora Server WG | Fedora KDE WG | Fedora QA Team
Fedora Infrastructure Apprentice
FAS: dmossor IRC: danofsatx
San Antonio, Texas, USA

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


[libvirt] [PATCH 04/13] qemu: Simplify qemuDomainGetNumaParameters by using virDomainObjGetOneDef

2015-06-15 Thread Peter Krempa
---
 src/qemu/qemu_driver.c | 20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dde94e8..e8b2be3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10277,36 +10277,24 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
 int *nparams,
 unsigned int flags)
 {
-virQEMUDriverPtr driver = dom-conn-privateData;
 size_t i;
 virDomainObjPtr vm = NULL;
-virDomainDefPtr persistentDef = NULL;
 virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
 char *nodeset = NULL;
 int ret = -1;
-virCapsPtr caps = NULL;
 virDomainDefPtr def = NULL;

 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG |
   VIR_TYPED_PARAM_STRING_OKAY, -1);

-/* We blindly return a string, and let libvirt.c and
- * remote_driver.c do the filtering on behalf of older clients
- * that can't parse it.  */
-flags = ~VIR_TYPED_PARAM_STRING_OKAY;
-
 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;

 if (virDomainGetNumaParametersEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;

-if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto cleanup;
-
-if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
-persistentDef)  0)
+if (!(def = virDomainObjGetOneDef(vm, flags)))
 goto cleanup;

 if ((*nparams) == 0) {
@@ -10315,11 +10303,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
 goto cleanup;
 }

-if (flags  VIR_DOMAIN_AFFECT_CONFIG)
-def = persistentDef;
-else
-def = vm-def;
-
 for (i = 0; i  QEMU_NB_NUMA_PARAM  i  *nparams; i++) {
 virMemoryParameterPtr param = params[i];

@@ -10357,7 +10340,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
  cleanup:
 VIR_FREE(nodeset);
 virDomainObjEndAPI(vm);
-virObjectUnref(caps);
 return ret;
 }

-- 
2.4.1

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


[libvirt] [PATCH 02/13] conf: Introduce helper to help getting correct def for getter functions

2015-06-15 Thread Peter Krempa
virDomainObjGetOneDef will help to retrieve the correct definition
pointer from @vm in cases where VIR_DOMAIN_AFFECT_LIVE and
VIR_DOMAIN_AFFECT_CONFIG are mutually exclusive. The function simply
returns the correct pointer. This similarly to virDomainObjGetDefs will
greatly simplify the code.
---
 src/conf/domain_conf.c   | 36 
 src/conf/domain_conf.h   |  1 +
 src/libvirt_private.syms |  1 +
 3 files changed, 38 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3cc182b..35e1cb4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2948,6 +2948,42 @@ virDomainObjGetDefs(virDomainObjPtr vm,
 }


+/**
+ * virDomainObjGetOneDef:
+ *
+ * @vm: Domain object
+ * @flags: for virDomainModificationImpact
+ *
+ * Helper function to resolve @flags and return the correct domain pointer
+ * object. This function returns one of @vm-def or @vm-persistentDef
+ * according to @flags. This helper should be used only in APIs that guarantee
+ * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE,
+ * VIR_DOMAIN_AFFECT_CONFIG.
+ *
+ * Returns the correct definition pointer or NULL on error.
+ */
+virDomainDefPtr
+virDomainObjGetOneDef(virDomainObjPtr vm,
+  unsigned int flags)
+{
+if (flags  VIR_DOMAIN_AFFECT_LIVE  flags  VIR_DOMAIN_AFFECT_CONFIG) {
+virReportInvalidArg(ctl, %s,
+_(Flags 'VIR_DOMAIN_AFFECT_LIVE' and 
+  'VIR_DOMAIN_AFFECT_CONFIG' are mutually 
+  exclusive));
+return NULL;
+}
+
+if (virDomainObjUpdateModificationImpact(vm, flags)  0)
+return NULL;
+
+if (virDomainObjIsActive(vm)  flags  VIR_DOMAIN_AFFECT_CONFIG)
+return vm-newDef;
+else
+return vm-def;
+}
+
+
 /*
  * The caller must hold a lock on the driver owning 'doms',
  * and must also have locked 'dom', to ensure no one else
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba17a8d..db49d46 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2553,6 +2553,7 @@ int virDomainObjGetDefs(virDomainObjPtr vm,
 unsigned int flags,
 virDomainDefPtr *liveDef,
 virDomainDefPtr *persDef);
+virDomainDefPtr virDomainObjGetOneDef(virDomainObjPtr vm, unsigned int flags);

 int
 virDomainLiveConfigHelperMethod(virCapsPtr caps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dc8a52d..858c00f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -385,6 +385,7 @@ virDomainObjEndAPI;
 virDomainObjFormat;
 virDomainObjGetDefs;
 virDomainObjGetMetadata;
+virDomainObjGetOneDef;
 virDomainObjGetPersistentDef;
 virDomainObjGetState;
 virDomainObjListAdd;
-- 
2.4.1

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


[libvirt] [PATCH 06/13] qemu: Simplify qemuDomainGetEmulatorPinInfo by using virDomainObjGetOneDef

2015-06-15 Thread Peter Krempa
virDomainObjGetOneDef is simpler to use than virDomainObjGetDefs
---
 src/qemu/qemu_driver.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f3c53f5..c878409 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5400,7 +5400,6 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
 {
 virDomainObjPtr vm = NULL;
 virDomainDefPtr def;
-virDomainDefPtr targetDef;
 int ret = -1;
 int hostcpus;
 virBitmapPtr cpumask = NULL;
@@ -5415,19 +5414,16 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
 if (virDomainGetEmulatorPinInfoEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;

-if (virDomainObjGetDefs(vm, flags, def, targetDef)  0)
+if (!(def = virDomainObjGetOneDef(vm, flags)))
 goto cleanup;

-if (def)
-targetDef = def;
-
 if ((hostcpus = nodeGetCPUCount())  0)
 goto cleanup;

-if (targetDef-cputune.emulatorpin) {
-cpumask = targetDef-cputune.emulatorpin;
-} else if (targetDef-cpumask) {
-cpumask = targetDef-cpumask;
+if (def-cputune.emulatorpin) {
+cpumask = def-cputune.emulatorpin;
+} else if (def-cpumask) {
+cpumask = def-cpumask;
 } else {
 if (!(bitmap = virBitmapNew(hostcpus)))
 goto cleanup;
-- 
2.4.1

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


[libvirt] [PATCHv2 01/13] conf: Fix virDomainObjGetDefs when getting persistent config on a live vm

2015-06-15 Thread Peter Krempa
If @flags contains only VIR_DOMAIN_AFFECT_CONFIG and @vm is active, the
function would return the active config rather than the persistent one
that it should return. This happened due to the fact that
virDomainObjGetDefs was checking the updated flags which may not contain
VIR_DOMAIN_AFFECT_LIVE if it is not requested even if @vm is active.

Additionally the function would not take the flags into account when
setting the pointers which was later used to determine whether the code
needs to update the given configuration.

The mistake was caught by the virt-test suite.
---
Version 2 actually fixes the function.

 src/conf/domain_conf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca55981..3cc182b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2933,11 +2933,11 @@ virDomainObjGetDefs(virDomainObjPtr vm,
 if (virDomainObjUpdateModificationImpact(vm, flags)  0)
 return -1;

-if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-if (liveDef)
+if (virDomainObjIsActive(vm)) {
+if (liveDef  (flags  VIR_DOMAIN_AFFECT_LIVE))
 *liveDef = vm-def;

-if (persDef)
+if (persDef  (flags  VIR_DOMAIN_AFFECT_CONFIG))
 *persDef = vm-newDef;
 } else {
 if (persDef)
-- 
2.4.1

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


[libvirt] [PATCH 12/13] qemu: 'privileged' flag is not really configuration

2015-06-15 Thread Peter Krempa
The privileged flag will not change while the configuration might
change. Make the 'privileged' flag member of the driver again and mark
it immutable. Should that ever change add an accessor that will group
reads of the state.
---
 src/qemu/qemu_cgroup.c   | 13 -
 src/qemu/qemu_command.c  |  9 +
 src/qemu/qemu_conf.c |  7 ++-
 src/qemu/qemu_conf.h |  5 -
 src/qemu/qemu_domain.c   |  4 ++--
 src/qemu/qemu_driver.c   | 36 +---
 tests/qemuxml2argvtest.c |  4 ++--
 7 files changed, 36 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 7d1f009..8ed74ee 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -714,7 +714,7 @@ qemuInitCgroup(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

-if (!cfg-privileged)
+if (!virQEMUDriverIsPrivileged(driver))
 goto done;

 if (!virCgroupAvailable())
@@ -745,7 +745,7 @@ qemuInitCgroup(virQEMUDriverPtr driver,

 if (virCgroupNewMachine(vm-def-name,
 qemu,
-cfg-privileged,
+true,
 vm-def-uuid,
 NULL,
 vm-pid,
@@ -844,7 +844,7 @@ qemuConnectCgroup(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 int ret = -1;

-if (!cfg-privileged)
+if (!virQEMUDriverIsPrivileged(driver))
 goto done;

 if (!virCgroupAvailable())
@@ -1247,22 +1247,17 @@ qemuRemoveCgroup(virQEMUDriverPtr driver,
  virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm-privateData;
-virQEMUDriverConfigPtr cfg;

 if (priv-cgroup == NULL)
 return 0; /* Not supported, so claim success */

-cfg = virQEMUDriverGetConfig(driver);
-
 if (virCgroupTerminateMachine(vm-def-name,
   qemu,
-  cfg-privileged)  0) {
+  virQEMUDriverIsPrivileged(driver))  0) {
 if (!virCgroupNewIgnoreError())
 VIR_DEBUG(Failed to terminate cgroup for %s, vm-def-name);
 }

-virObjectUnref(cfg);
-
 return virCgroupRemove(priv-cgroup);
 }

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3886b4f..a51a3f6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -354,7 +354,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,

 if (net-backend.tap) {
 tunpath = net-backend.tap;
-if (!cfg-privileged) {
+if (!(virQEMUDriverIsPrivileged(driver))) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(cannot use custom tap device in session mode));
 goto cleanup;
@@ -381,7 +381,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
 }

-if (cfg-privileged) {
+if (virQEMUDriverIsPrivileged(driver)) {
 if (virNetDevTapCreateInBridgePort(brname, net-ifname, net-mac,
def-uuid, tunpath, tapfd, 
*tapfdSize,

virDomainNetGetActualVirtPortProfile(net),
@@ -8284,7 +8284,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
 /* network and bridge use a tap device, and direct uses a
  * macvtap device
  */
-if (cfg-privileged  nicindexes  nnicindexes  net-ifname) {
+if (virQEMUDriverIsPrivileged(driver)  nicindexes  nnicindexes 
+net-ifname) {
 if (virNetDevGetIndex(net-ifname, nicindex)  0 ||
 VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex)  0)
 goto cleanup;
@@ -8764,7 +8765,7 @@ qemuBuildCommandLine(virConnectPtr conn,

 emulator = def-emulator;

-if (!cfg-privileged) {
+if (!virQEMUDriverIsPrivileged(driver)) {
 /* If we have no cgroups then we can have no tunings that
  * require them */

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 16ae6ab..d521886 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -164,7 +164,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 if (!(cfg = virObjectNew(virQEMUDriverConfigClass)))
 return NULL;

-cfg-privileged = privileged;
 cfg-uri = privileged ? qemu:///system : qemu:///session;

 if (privileged) {
@@ -873,6 +872,12 @@ virQEMUDriverConfigPtr 
virQEMUDriverGetConfig(virQEMUDriverPtr driver)
 return conf;
 }

+bool
+virQEMUDriverIsPrivileged(virQEMUDriverPtr driver)
+{
+return driver-privileged;
+}
+
 virDomainXMLOptionPtr
 virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver)
 {
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 2ba4ce7..b74c283 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h

[libvirt] [PATCH 00/13] vCPU pinning and related refactors - Part 1.5

2015-06-15 Thread Peter Krempa
Few more fixes to the Part 1 and related stuff. This is a continuation and fix
to stuff done in Part 1.

Peter Krempa (13):
  conf: Fix virDomainObjGetDefs when getting persistent config on a live
vm
  conf: Introduce helper to help getting correct def for getter
functions
  qemu: Simplify qemuDomainGetInterfaceParameters by using
virDomainObjGetOneDef
  qemu: Simplify qemuDomainGetNumaParameters by using
virDomainObjGetOneDef
  qemu: Simplify qemuDomainGetVcpuPinInfo by using virDomainObjGetOneDef
  qemu: Simplify qemuDomainGetEmulatorPinInfo by using
virDomainObjGetOneDef
  qemu: Simplify qemuDomainGetVcpusFlags by using virDomainObjGetOneDef
  qemu: Simplify qemuDomainSetInterfaceParameters by using
virDomainObjGetDefs
  qemu: Refactor qemuDomainSetNumaParameters
  qemu: Refactor qemuDomainGetMemoryParameters
  qemu: Reuse virDomainObjGetDefs in qemuDomainGetMemoryParameters
  qemu: 'privileged' flag is not really configuration
  conf: Move vcpu info parsing code into a separate function

 src/conf/domain_conf.c   | 174 -
 src/conf/domain_conf.h   |   1 +
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_cgroup.c   |  13 +-
 src/qemu/qemu_command.c  |   9 +-
 src/qemu/qemu_conf.c |   7 +-
 src/qemu/qemu_conf.h |   5 +-
 src/qemu/qemu_domain.c   |   4 +-
 src/qemu/qemu_driver.c   | 332 ++-
 tests/qemuxml2argvtest.c |   4 +-
 10 files changed, 241 insertions(+), 309 deletions(-)

-- 
2.4.1

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


[libvirt] [PATCH 11/13] qemu: Reuse virDomainObjGetDefs in qemuDomainGetMemoryParameters

2015-06-15 Thread Peter Krempa
Simplify the code by restructuring control flow and reusing the better
helper.
---
 src/qemu/qemu_driver.c | 27 ---
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f13e243..e09bb70 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9936,7 +9936,6 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,
 virDomainObjPtr vm = NULL;
 virDomainDefPtr persistentDef = NULL;
 int ret = -1;
-virCapsPtr caps = NULL;
 qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
 unsigned long long swap_hard_limit, mem_hard_limit, mem_soft_limit;
@@ -9945,9 +9944,6 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_CONFIG |
   VIR_TYPED_PARAM_STRING_OKAY, -1);

-/* We don't return strings, and thus trivially support this flag.  */
-flags = ~VIR_TYPED_PARAM_STRING_OKAY;
-
 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;

@@ -9963,21 +9959,9 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,
 goto cleanup;
 }

-if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+if (virDomainObjGetDefs(vm, flags, NULL, persistentDef)  0)
 goto cleanup;

-if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
-persistentDef)  0)
-goto cleanup;
-
-if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-if (!virCgroupHasController(priv-cgroup, 
VIR_CGROUP_CONTROLLER_MEMORY)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   %s, _(cgroup memory controller is not mounted));
-goto cleanup;
-}
-}
-
 if ((*nparams) == 0) {
 /* Current number of memory parameters supported by cgroups */
 *nparams = QEMU_NB_MEM_PARAM;
@@ -9985,11 +9969,17 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,
 goto cleanup;
 }

-if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
+if (persistentDef) {
 mem_hard_limit = persistentDef-mem.hard_limit;
 mem_soft_limit = persistentDef-mem.soft_limit;
 swap_hard_limit = persistentDef-mem.swap_hard_limit;
 } else {
+if (!virCgroupHasController(priv-cgroup, 
VIR_CGROUP_CONTROLLER_MEMORY)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(cgroup memory controller is not mounted));
+goto cleanup;
+}
+
 if (virCgroupGetMemoryHardLimit(priv-cgroup, mem_hard_limit)  0)
 goto cleanup;

@@ -10014,7 +10004,6 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,

  cleanup:
 virDomainObjEndAPI(vm);
-virObjectUnref(caps);
 virObjectUnref(cfg);
 return ret;
 }
-- 
2.4.1

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


[libvirt] [PATCH 13/13] conf: Move vcpu info parsing code into a separate function

2015-06-15 Thread Peter Krempa
---
 src/conf/domain_conf.c | 132 -
 1 file changed, 76 insertions(+), 56 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35e1cb4..454d6cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14082,6 +14082,81 @@ virDomainThreadSchedParse(xmlNodePtr node,
 return -1;
 }

+
+static int
+virDomainVcpuParse(virDomainDefPtr def,
+   xmlXPathContextPtr ctxt)
+{
+int n;
+char *tmp = NULL;
+int ret = -1;
+
+if ((n = virXPathUInt(string(./vcpu[1]), ctxt, def-maxvcpus))  0) {
+if (n == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(maximum vcpus count must be an integer));
+goto cleanup;
+}
+
+def-maxvcpus = 1;
+}
+
+if ((n = virXPathUInt(string(./vcpu[1]/@current), ctxt, def-vcpus))  
0) {
+if (n == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(current vcpus count must be an integer));
+goto cleanup;
+}
+
+def-vcpus = def-maxvcpus;
+}
+
+if (def-maxvcpus  def-vcpus) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(maxvcpus must not be less than current vcpus 
+ (%u  %u)), def-maxvcpus, def-vcpus);
+goto cleanup;
+}
+
+tmp = virXPathString(string(./vcpu[1]/@placement), ctxt);
+if (tmp) {
+if ((def-placement_mode =
+ virDomainCpuPlacementModeTypeFromString(tmp))  0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(Unsupported CPU placement mode '%s'),
+tmp);
+ goto cleanup;
+}
+VIR_FREE(tmp);
+} else {
+def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC;
+}
+
+if (def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
+tmp = virXPathString(string(./vcpu[1]/@cpuset), ctxt);
+if (tmp) {
+if (virBitmapParse(tmp, 0, def-cpumask,
+   VIR_DOMAIN_CPUMASK_LEN)  0)
+goto cleanup;
+
+if (virBitmapIsAllClear(def-cpumask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'cpuset': %s), tmp);
+goto cleanup;
+}
+
+VIR_FREE(tmp);
+}
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(tmp);
+
+return ret;
+}
+
 static virDomainDefPtr
 virDomainDefParseXML(xmlDocPtr xml,
  xmlNodePtr root,
@@ -14378,63 +14453,8 @@ virDomainDefParseXML(xmlDocPtr xml,
   def-mem.swap_hard_limit)  0)
 goto error;

-if ((n = virXPathUInt(string(./vcpu[1]), ctxt, def-maxvcpus))  0) {
-if (n == -2) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(maximum vcpus count must be an integer));
-goto error;
-}
-
-def-maxvcpus = 1;
-}
-
-if ((n = virXPathUInt(string(./vcpu[1]/@current), ctxt, def-vcpus))  
0) {
-if (n == -2) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(current vcpus count must be an integer));
-goto error;
-}
-
-def-vcpus = def-maxvcpus;
-}
-
-if (def-maxvcpus  def-vcpus) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(maxvcpus must not be less than current vcpus 
- (%u  %u)), def-maxvcpus, def-vcpus);
+if (virDomainVcpuParse(def, ctxt)  0)
 goto error;
-}
-
-tmp = virXPathString(string(./vcpu[1]/@placement), ctxt);
-if (tmp) {
-if ((def-placement_mode =
- virDomainCpuPlacementModeTypeFromString(tmp))  0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-_(Unsupported CPU placement mode '%s'),
-tmp);
- goto error;
-}
-VIR_FREE(tmp);
-} else {
-def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC;
-}
-
-if (def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
-tmp = virXPathString(string(./vcpu[1]/@cpuset), ctxt);
-if (tmp) {
-if (virBitmapParse(tmp, 0, def-cpumask,
-   VIR_DOMAIN_CPUMASK_LEN)  0)
-goto error;
-
-if (virBitmapIsAllClear(def-cpumask)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _(Invalid value of 'cpuset': %s), tmp);
-goto error;
-}
-
-VIR_FREE(tmp);
-}
-}

 /* Optional - iothreads */
 tmp = virXPathString(string(./iothreads[1]), ctxt);
-- 
2.4.1

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


[libvirt] [PATCH] conf: Adjust invalid secrettype setting during parse

2015-06-15 Thread John Ferlan
Commit id '1feaccf0' attempted to handle an empty secrettype value; however,
it made a mistake by processing the secretType as if it was the original
secrettype string.  The 'secretType' is actually whether 'usage' or 'uuid'
was used.

Thus adjust part of the change to make the same check for def-src-type !=
VIR_STORAGE_TYPE_VOLUME before setting auth_secret_usage from the
secrettype field.

Luckily the aforementioned commits misdeed would be overwritten by the
call to virStorageTranslateDiskSourcePool

Signed-off-by: John Ferlan jfer...@redhat.com
---

Follow up to my recent change and response, see:

http://www.redhat.com/archives/libvir-list/2015-June/msg00666.html

 src/conf/domain_conf.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca55981..c31edf5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6571,17 +6571,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur-name, BAD_CAST auth)) {
 if (!(authdef = virStorageAuthDefParse(node-doc, cur)))
 goto error;
-/* Shared processing code with storage pools can leave
- * this empty, but disk formatting uses it as does command
- * creation - so use the secretType to attempt to fill it in.
+/* Disk volume types won't have the secrettype filled in until
+ * after virStorageTranslateDiskSourcePool is run
  */
-if (!authdef-secrettype) {
-const char *secrettype =
-virSecretUsageTypeToString(authdef-secretType);
-if (VIR_STRDUP(authdef-secrettype, secrettype)  0)
-goto error;
-}
-if ((auth_secret_usage =
+if (def-src-type != VIR_STORAGE_TYPE_VOLUME 
+(auth_secret_usage =
  virSecretUsageTypeFromString(authdef-secrettype))  0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(invalid secret type %s),
-- 
2.1.0

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


Re: [libvirt] [PATCH 1/2] storage: Fix the schema and add tests for cifs pool

2015-06-15 Thread Peter Krempa
On Wed, Jun 03, 2015 at 13:06:57 -0400, John Ferlan wrote:
 Commit id '887dd362' added support for a netfs pool format type 'cifs'
 and 'gluster' in order to add rng support for Samba and glusterfs netfs
 pools. Originally, the CIFS type support was added as part of commit
 id '61fb6979'. Eventually commit id 'b325be12' fixed the gluster rng
 definition to match expectations.
 
 As it turns out the CIFS rng needed a similar change since the directory
 path is not an absDirPath, rather just a dirPath will be required.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/schemas/storagepool.rng| 24 +++-
  tests/storagepoolxml2xmlin/pool-netfs-cifs.xml  | 12 
  tests/storagepoolxml2xmlout/pool-netfs-cifs.xml | 15 +++
  tests/storagepoolxml2xmltest.c  |  1 +
  4 files changed, 51 insertions(+), 1 deletion(-)
  create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-cifs.xml
  create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-cifs.xml
 
 diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
 index db6ff49..d6bf772 100644
 --- a/docs/schemas/storagepool.rng
 +++ b/docs/schemas/storagepool.rng
 @@ -314,6 +314,15 @@
  /element
/define
  
 +  define name='sourceinfonetfscifs'

This is equivalent to the gluster one. How about renaming the gluster
one to a more generic name ... and

 +element name='dir'
 +  attribute name='path'
 +ref name='dirPath'/
 +  /attribute
 +  empty/
 +/element
 +  /define
 +
define name='sourceinfonetfsgluster'
  element name='dir'
attribute name='path'
 @@ -400,7 +409,6 @@
choice
  valueauto/value
  valuenfs/value
 -valuecifs/value
/choice
  /attribute
/element
 @@ -488,6 +496,20 @@
  group
interleave
  ref name='sourceinfohost'/
 +ref name='sourceinfonetfscifs'/
 +element name='format'
 +  attribute name='type'
 +valuecifs/value

since the whole block is identical you could just add 
choice
  valuecifs/value
  valueglusterfs/value
/choice

here to avoid duplicating the whole section.

 +  /attribute
 +/element
 +optional
 +ref name='sourceinfovendor'/
 +/optional
 +  /interleave
 +/group
 +group
 +  interleave
 +ref name='sourceinfohost'/
  ref name='sourceinfonetfsgluster'/
  element name='format'
attribute name='type'
 diff --git a/tests/storagepoolxml2xmlin/pool-netfs-cifs.xml 
 b/tests/storagepoolxml2xmlin/pool-netfs-cifs.xml

The test section looks good to me. ACK if you avoid the duplication.

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 2/2] storage: Generate correct parameters for CIFS

2015-06-15 Thread Peter Krempa
On Wed, Jun 03, 2015 at 13:06:58 -0400, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1186969
 
 When generating the path to the dir for a CIFS/Samba driver, the code
 would generate a source path for the mount using %s:%s while the
 mount.cifs expects to see //%s/%s. So check for the cifsfs and
 format the source path appropriately.
 
 Additionally, since there is no means to authenticate, the mount
 needs a -o guest on the command line in order to anonymously mount
 the Samba directory.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/formatstorage.html.in   |  7 +--
  docs/storage.html.in |  3 ++-
  src/storage/storage_backend_fs.c | 29 -
  3 files changed, 31 insertions(+), 8 deletions(-)
 
 diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
 index 17558f8..b6f4361 100644
 --- a/docs/formatstorage.html.in
 +++ b/docs/formatstorage.html.in
 @@ -124,11 +124,14 @@
  span class=sinceSince 0.4.1/span/dd
dtcodedir/code/dt
ddProvides the source for pools backed by directories (pool
 -type codedir/code), or optionally to select a subdirectory
 +types codedir/code, codenetfs/code, codegluster/code),
 +or optionally to select a subdirectory
  within a pool that resembles a filesystem (pool
  type codegluster/code). May
  only occur once. Contains a single attribute codepath/code
 -which is the fully qualified path to the backing directory.
 +which is the fully qualified path to the backing directory or
 +for a codenetfs/code pool type using codeformat/code
 +type cifs, the path to the Samba share without the leading slash.
  span class=sinceSince 0.4.1/span/dd
dtcodeadapter/code/dt
ddProvides the source for pools backed by SCSI adapters (pool
 diff --git a/docs/storage.html.in b/docs/storage.html.in
 index 92e9ae7..0b467d5 100644
 --- a/docs/storage.html.in
 +++ b/docs/storage.html.in
 @@ -291,7 +291,8 @@
  the a href=#StorageBackendGlustergluster/a pool.)
/li
li
 -codecifs/code - use the SMB (samba) or CIFS file system
 +codecifs/code - use the SMB (samba) or CIFS file system.
 +The mount will use -o guest to mount the directory anonymously.
/li
  /ul
  
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index 337b8d3..6ba698c 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -388,6 +388,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr 
 pool)
  pool-def-source.format == VIR_STORAGE_POOL_NETFS_AUTO);
  bool glusterfs = (pool-def-type == VIR_STORAGE_POOL_NETFS 
pool-def-source.format == 
 VIR_STORAGE_POOL_NETFS_GLUSTERFS);
 +bool cifsfs = (pool-def-type == VIR_STORAGE_POOL_NETFS 
 +   pool-def-source.format == VIR_STORAGE_POOL_NETFS_CIFS);

@cifsfs can be true only if pool-def-type == VIR_STORAGE_POOL_NETFS.

  virCommandPtr cmd = NULL;
  int ret = -1;
  int rc;
 @@ -427,11 +429,17 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr 
 pool)
  }
  
  if (pool-def-type == VIR_STORAGE_POOL_NETFS) {
 -if (virAsprintf(src, %s:%s,
 -pool-def-source.hosts[0].name,
 -pool-def-source.dir) == -1)
 -return -1;
 -
 +if (pool-def-source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
 +if (virAsprintf(src, //%s/%s,
 +pool-def-source.hosts[0].name,
 +pool-def-source.dir) == -1)
 +return -1;
 +} else {
 +if (virAsprintf(src, %s:%s,
 +pool-def-source.hosts[0].name,
 +pool-def-source.dir) == -1)
 +return -1;
 +}
  } else {
  if (VIR_STRDUP(src, pool-def-source.devices[0].path)  0)
  return -1;
 @@ -453,6 +461,17 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr 
 pool)
 direct-io-mode=1,
 pool-def-target.path,
 NULL);
 +else if (cifsfs)
 +cmd = virCommandNewArgList(MOUNT,
 +   -t,
 +   (pool-def-type == VIR_STORAGE_POOL_FS ?

... so the first part of this ternary can't ever be true here.

 +
 virStoragePoolFormatFileSystemTypeToString(pool-def-source.format) :
 +
 virStoragePoolFormatFileSystemNetTypeToString(pool-def-source.format)),
 +   src,
 +   pool-def-target.path,
 +   -o,
 +   guest,
 +   

[libvirt] [PATCH 0/4] parallels: Implementation of attach/detach network devices and small network fixes

2015-06-15 Thread Mikhail Feoktistov
Mikhail Feoktistov (4):
  parallels: Fix initialization of buflen variable in each loop
iteration
  parallels: Fix false error messages in libvirt log
  parallels: Switch on DHCP for newly created network adapters
  parallels: implementation of attach/detach network devices

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


[libvirt] [PATCH 3/4] parallels: Switch on DHCP for newly created network adapters

2015-06-15 Thread Mikhail Feoktistov
Let network adapter use DHCP server to get network configuration.
To do this we use PrlVmDevNet_SetConfigureWithDhcp to enable it and
PrlVmDevNet_SetAutoApply to makes necessary settings within guest OS
In linux case it creates network startup scripts
/etc/sysconfig/network-scripts/ifcfg-ethN and fills it with necessary
parameters.
---
 src/parallels/parallels_sdk.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 1d982c5..4c3d1eb 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2815,6 +2815,12 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
 pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
 prlsdkCheckRetGoto(pret, cleanup);
 
+pret = PrlVmDevNet_SetConfigureWithDhcp(sdknet, true);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDevNet_SetAutoApply(sdknet, true);
+prlsdkCheckRetGoto(pret, cleanup);
+
 if (isCt) {
 if (net-model)
  VIR_WARN(Setting network adapter for containers is not 
-- 
1.7.1

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


[libvirt] [PATCH 4/4] parallels: implementation of attach/detach network devices

2015-06-15 Thread Mikhail Feoktistov
In this patch we add VIR_DOMAIN_DEVICE_NET handlers implementation
for domainAttachDevice and domainDetachDevice callbacks.

As soon as we don't support this operation for hypervisor type domains,
we implement this functionality for containers only.

In detach procedure we find network device by MAC address.
Because PrlVmDevNet_GetMacAddress() returns MAC as a UTF-8 encoded
null-terminated string, we use memcmp() to compare it.
Also we remove corresponding virtual network by prlsdkDelNetAdapter call.
---
 src/parallels/parallels_driver.c |   16 +
 src/parallels/parallels_sdk.c|  127 ++
 src/parallels/parallels_sdk.h|4 +
 3 files changed, 147 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 706229d..0009127 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1117,6 +1117,14 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr 
dom, const char *xml,
 goto cleanup;
 }
 break;
+case VIR_DOMAIN_DEVICE_NET:
+ret = prlsdkAttachNet(privdom, privconn, dev-data.net);
+if (ret) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(network attach failed));
+goto cleanup;
+}
+break;
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(device type '%s' cannot be attached),
@@ -1186,6 +1194,14 @@ static int parallelsDomainDetachDeviceFlags(virDomainPtr 
dom, const char *xml,
 goto cleanup;
 }
 break;
+case VIR_DOMAIN_DEVICE_NET:
+ret = prlsdkDetachNet(privdom, privconn, dev-data.net);
+if (ret) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(network detach failed));
+goto cleanup;
+}
+break;
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(device type '%s' cannot be detached),
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 4c3d1eb..8fec34c 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2914,6 +2914,133 @@ static void prlsdkDelNet(parallelsConnPtr privconn, 
virDomainNetDefPtr net)
 PrlHandle_Free(vnet);
 }
 
+int prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, 
virDomainNetDefPtr net)
+{
+int ret = -1;
+parallelsDomObjPtr privdom = dom-privateData;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
+
+if (!IS_CT(dom-def)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(network device cannot be attached));
+goto cleanup;
+}
+
+job = PrlVm_BeginEdit(privdom-sdkdom);
+if (PRL_FAILED(waitJob(job)))
+goto cleanup;
+
+ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def));
+if (ret == 0) {
+job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE);
+if (PRL_FAILED(waitJob(job))) {
+ret = -1;
+goto cleanup;
+}
+}
+
+ cleanup:
+return ret;
+}
+
+static int
+prlsdkGetNetIndex(PRL_HANDLE sdkdom, virDomainNetDefPtr net)
+{
+int idx = -1;
+PRL_RESULT pret;
+PRL_UINT32 netCount;
+PRL_UINT32 i;
+PRL_HANDLE adapter = PRL_INVALID_HANDLE;
+PRL_UINT32 len;
+char adapterMac[PRL_MAC_STRING_BUFNAME];
+char netMac[PRL_MAC_STRING_BUFNAME];
+
+prlsdkFormatMac(net-mac, netMac);
+pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, netCount);
+prlsdkCheckRetGoto(pret, cleanup);
+
+for (i = 0; i  netCount; ++i) {
+
+pret = PrlVmCfg_GetNetAdapter(sdkdom, i, adapter);
+prlsdkCheckRetGoto(pret, cleanup);
+
+len = sizeof(adapterMac);
+memset(adapterMac, 0, sizeof(adapterMac));
+pret = PrlVmDevNet_GetMacAddress(adapter, adapterMac, len);
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (memcmp(adapterMac, netMac, PRL_MAC_STRING_BUFNAME)) {
+
+PrlHandle_Free(adapter);
+adapter = PRL_INVALID_HANDLE;
+continue;
+}
+
+idx = i;
+break;
+}
+
+ cleanup:
+PrlHandle_Free(adapter);
+return idx;
+}
+
+static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx)
+{
+int ret = -1;
+PRL_RESULT pret;
+PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
+
+pret = PrlVmCfg_GetNetAdapter(sdkdom, idx, sdknet);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDev_Remove(sdknet);
+prlsdkCheckRetGoto(pret, cleanup);
+
+ret = 0;
+
+ cleanup:
+PrlHandle_Free(sdknet);
+return ret;
+}
+
+int prlsdkDetachNet(virDomainObjPtr dom, parallelsConnPtr privconn, 
virDomainNetDefPtr net)
+{
+int ret = -1, idx = -1;
+parallelsDomObjPtr privdom = dom-privateData;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
+
+if (!IS_CT(dom-def)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,

[libvirt] [PATCH 2/4] parallels: Fix false error messages in libvirt log

2015-06-15 Thread Mikhail Feoktistov
There was many errors in libvirt.log caused by prlsdkDelNet function because
job variable was always initialized as PRL_INVALID_HANDLE
In this patch job variable gets return value of PrlSrv_DeleteVirtualNetwork 
function()
---
 src/parallels/parallels_sdk.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index d27bac9..1d982c5 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2900,7 +2900,7 @@ static void prlsdkDelNet(parallelsConnPtr privconn, 
virDomainNetDefPtr net)
 pret = PrlVirtNet_SetNetworkId(vnet, net-data.network.name);
 prlsdkCheckRetGoto(pret, cleanup);
 
-PrlSrv_DeleteVirtualNetwork(privconn-server, vnet, 0);
+job = PrlSrv_DeleteVirtualNetwork(privconn-server, vnet, 0);
 if (PRL_FAILED(pret = waitJob(job)))
 goto cleanup;
 
-- 
1.7.1

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


[libvirt] [PATCH 1/4] parallels: Fix initialization of buflen variable in each loop iteration

2015-06-15 Thread Mikhail Feoktistov
We need to initialize buflen every time when we get network adapter's
friendly name because we call PrlVmDev_GetFriendlyName in a loop
---
 src/parallels/parallels_sdk.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 104c905..d27bac9 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -3150,6 +3150,7 @@ prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr 
disk)
 pret = PrlVmCfg_GetHardDisk(sdkdom, i, hdd);
 prlsdkCheckRetGoto(pret, cleanup);
 
+buflen = 0;
 pret = PrlVmDev_GetFriendlyName(hdd, 0, buflen);
 prlsdkCheckRetGoto(pret, cleanup);
 
-- 
1.7.1

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


[libvirt] [PATCH 08/13] qemu: Simplify qemuDomainSetInterfaceParameters by using virDomainObjGetDefs

2015-06-15 Thread Peter Krempa
---
 src/qemu/qemu_driver.c | 40 
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2cb0215..ceadc31 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11246,12 +11246,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 virQEMUDriverPtr driver = dom-conn-privateData;
 size_t i;
 virDomainObjPtr vm = NULL;
-virDomainDefPtr persistentDef = NULL;
+virDomainDefPtr def;
+virDomainDefPtr persistentDef;
 int ret = -1;
 virDomainNetDefPtr net = NULL, persistentNet = NULL;
 virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
-virCapsPtr caps = NULL;
 bool inboundSpecified = false, outboundSpecified = false;

 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -11280,31 +11280,24 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 if (virDomainSetInterfaceParametersEnsureACL(dom-conn, vm-def, flags)  
0)
 goto cleanup;

-if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
 goto cleanup;

-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
+if (virDomainObjGetDefs(vm, flags, def, persistentDef)  0)
 goto cleanup;

-if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
-persistentDef)  0)
+if (def 
+!(net = virDomainNetFind(vm-def, device))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(Can't find device %s), device);
 goto endjob;
-
-if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-net = virDomainNetFind(vm-def, device);
-if (!net) {
-virReportError(VIR_ERR_INVALID_ARG,
-   _(Can't find device %s), device);
-goto endjob;
-}
 }
-if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
-persistentNet = virDomainNetFind(persistentDef, device);
-if (!persistentNet) {
-virReportError(VIR_ERR_INVALID_ARG,
-   _(Can't find device %s), device);
-goto endjob;
-}
+
+if (persistentDef 
+!(persistentNet = virDomainNetFind(persistentDef, device))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(Can't find device %s), device);
+goto endjob;
 }

 if ((VIR_ALLOC(bandwidth)  0) ||
@@ -11340,7 +11333,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 if (!bandwidth-out-average)
 VIR_FREE(bandwidth-out);

-if (flags  VIR_DOMAIN_AFFECT_LIVE) {
+if (net) {
 if (VIR_ALLOC(newBandwidth)  0)
 goto endjob;

@@ -11392,7 +11385,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 goto endjob;
 }

-if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
+if (persistentNet) {
 if (!persistentNet-bandwidth) {
 persistentNet-bandwidth = bandwidth;
 bandwidth = NULL;
@@ -11426,7 +11419,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 virNetDevBandwidthFree(bandwidth);
 virNetDevBandwidthFree(newBandwidth);
 virDomainObjEndAPI(vm);
-virObjectUnref(caps);
 virObjectUnref(cfg);
 return ret;
 }
-- 
2.4.1

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


[libvirt] [PATCH 10/13] qemu: Refactor qemuDomainGetMemoryParameters

2015-06-15 Thread Peter Krempa
Replace the for loops with case inside with temp variables and a macro.
---
 src/qemu/qemu_driver.c | 99 +-
 1 file changed, 25 insertions(+), 74 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ca04d2c..f13e243 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9919,6 +9919,13 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
 return ret;
 }

+
+#define QEMU_ASSIGN_MEM_PARAM(index, name, value)  
\
+if (index  *nparams 
\
+virTypedParameterAssign(params[index], name, VIR_TYPED_PARAM_ULLONG,  
 \
+value)  0)
\
+goto cleanup
+
 static int
 qemuDomainGetMemoryParameters(virDomainPtr dom,
   virTypedParameterPtr params,
@@ -9926,13 +9933,13 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,
   unsigned int flags)
 {
 virQEMUDriverPtr driver = dom-conn-privateData;
-size_t i;
 virDomainObjPtr vm = NULL;
 virDomainDefPtr persistentDef = NULL;
 int ret = -1;
 virCapsPtr caps = NULL;
 qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
+unsigned long long swap_hard_limit, mem_hard_limit, mem_soft_limit;

 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG |
@@ -9979,85 +9986,28 @@ qemuDomainGetMemoryParameters(virDomainPtr dom,
 }

 if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
-for (i = 0; i  *nparams  i  QEMU_NB_MEM_PARAM; i++) {
-virMemoryParameterPtr param = params[i];
-unsigned long long value;
-
-switch (i) {
-case 0: /* fill memory hard limit here */
-value = persistentDef-mem.hard_limit;
-if (virTypedParameterAssign(param, 
VIR_DOMAIN_MEMORY_HARD_LIMIT,
-VIR_TYPED_PARAM_ULLONG, value)  0)
-goto cleanup;
-break;
-
-case 1: /* fill memory soft limit here */
-value = persistentDef-mem.soft_limit;
-if (virTypedParameterAssign(param, 
VIR_DOMAIN_MEMORY_SOFT_LIMIT,
-VIR_TYPED_PARAM_ULLONG, value)  0)
-goto cleanup;
-break;
-
-case 2: /* fill swap hard limit here */
-value = persistentDef-mem.swap_hard_limit;
-if (virTypedParameterAssign(param, 
VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
-VIR_TYPED_PARAM_ULLONG, value)  0)
-goto cleanup;
-break;
-
-/* coverity[dead_error_begin] */
-default:
-break;
-/* should not hit here */
-}
-}
-goto out;
-}
-
-for (i = 0; i  *nparams  i  QEMU_NB_MEM_PARAM; i++) {
-virTypedParameterPtr param = params[i];
-unsigned long long val = 0;
-
-switch (i) {
-case 0: /* fill memory hard limit here */
-if (virCgroupGetMemoryHardLimit(priv-cgroup, val)  0)
-goto cleanup;
-if (virTypedParameterAssign(param,
-VIR_DOMAIN_MEMORY_HARD_LIMIT,
-VIR_TYPED_PARAM_ULLONG, val)  0)
-goto cleanup;
-break;
+mem_hard_limit = persistentDef-mem.hard_limit;
+mem_soft_limit = persistentDef-mem.soft_limit;
+swap_hard_limit = persistentDef-mem.swap_hard_limit;
+} else {
+if (virCgroupGetMemoryHardLimit(priv-cgroup, mem_hard_limit)  0)
+goto cleanup;

-case 1: /* fill memory soft limit here */
-if (virCgroupGetMemorySoftLimit(priv-cgroup, val)  0)
-goto cleanup;
-if (virTypedParameterAssign(param,
-VIR_DOMAIN_MEMORY_SOFT_LIMIT,
-VIR_TYPED_PARAM_ULLONG, val)  0)
-goto cleanup;
-break;
+if (virCgroupGetMemorySoftLimit(priv-cgroup, mem_soft_limit)  0)
+goto cleanup;

-case 2: /* fill swap hard limit here */
-if (virCgroupGetMemSwapHardLimit(priv-cgroup, val)  0) {
-if (!virLastErrorIsSystemErrno(ENOENT) 
-!virLastErrorIsSystemErrno(EOPNOTSUPP))
-goto cleanup;
-val = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
-}
-if (virTypedParameterAssign(param,
-VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
-VIR_TYPED_PARAM_ULLONG, val)  0)
+if (virCgroupGetMemSwapHardLimit(priv-cgroup, swap_hard_limit)  0) {
+if 

[libvirt] [PATCH 03/13] qemu: Simplify qemuDomainGetInterfaceParameters by using virDomainObjGetOneDef

2015-06-15 Thread Peter Krempa
---
 src/qemu/qemu_driver.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d1f195c..dde94e8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11468,32 +11468,23 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
  int *nparams,
  unsigned int flags)
 {
-virQEMUDriverPtr driver = dom-conn-privateData;
 size_t i;
 virDomainObjPtr vm = NULL;
 virDomainDefPtr def = NULL;
-virDomainDefPtr persistentDef = NULL;
 virDomainNetDefPtr net = NULL;
 int ret = -1;
-virCapsPtr caps = NULL;

 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG |
   VIR_TYPED_PARAM_STRING_OKAY, -1);

-flags = ~VIR_TYPED_PARAM_STRING_OKAY;
-
 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;

 if (virDomainGetInterfaceParametersEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;

-if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto cleanup;
-
-if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
-persistentDef)  0)
+if (!(def = virDomainObjGetOneDef(vm, flags)))
 goto cleanup;

 if ((*nparams) == 0) {
@@ -11502,10 +11493,6 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
 goto cleanup;
 }

-def = persistentDef;
-if (!def)
-def = vm-def;
-
 net = virDomainNetFind(def, device);
 if (!net) {
 virReportError(VIR_ERR_INVALID_ARG,
@@ -11576,7 +11563,6 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,

  cleanup:
 virDomainObjEndAPI(vm);
-virObjectUnref(caps);
 return ret;
 }

-- 
2.4.1

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


[libvirt] [PATCH 09/13] qemu: Refactor qemuDomainSetNumaParameters

2015-06-15 Thread Peter Krempa
Use virDomainObjGetDefs and sanitize the control flow.
---
 src/qemu/qemu_driver.c | 64 ++
 1 file changed, 28 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ceadc31..ca04d2c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10131,11 +10131,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 {
 virQEMUDriverPtr driver = dom-conn-privateData;
 size_t i;
-virDomainDefPtr persistentDef = NULL;
+virDomainDefPtr def;
+virDomainDefPtr persistentDef;
 virDomainObjPtr vm = NULL;
 int ret = -1;
 virQEMUDriverConfigPtr cfg = NULL;
-virCapsPtr caps = NULL;
 qemuDomainObjPrivatePtr priv;
 virBitmapPtr nodeset = NULL;
 virDomainNumatuneMemMode config_mode;
@@ -10161,31 +10161,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 if (virDomainSetNumaParametersEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;

-if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto cleanup;
-
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
-goto cleanup;
-
-if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
-persistentDef)  0)
-goto endjob;
-
-if (!cfg-privileged 
-flags  VIR_DOMAIN_AFFECT_LIVE) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
-   _(NUMA tuning is not available in session mode));
-goto endjob;
-}
-
-if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-if (!virCgroupHasController(priv-cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
-   _(cgroup cpuset controller is not mounted));
-goto endjob;
-}
-}
-
 for (i = 0; i  nparams; i++) {
 virTypedParameterPtr param = params[i];

@@ -10195,26 +10170,44 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 if (mode  0 || mode = VIR_DOMAIN_NUMATUNE_MEM_LAST) {
 virReportError(VIR_ERR_INVALID_ARG,
_(unsupported numatune mode: '%d'), mode);
-goto endjob;
+goto cleanup;
 }

 } else if (STREQ(param-field, VIR_DOMAIN_NUMA_NODESET)) {
 if (virBitmapParse(param-value.s, 0, nodeset,
VIR_DOMAIN_CPUMASK_LEN)  0)
-goto endjob;
+goto cleanup;

 if (virBitmapIsAllClear(nodeset)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
_(Invalid nodeset of 'numatune': %s),
param-value.s);
-goto endjob;
+goto cleanup;
 }
 }
 }

-if (flags  VIR_DOMAIN_AFFECT_LIVE) {
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
+goto cleanup;
+
+if (virDomainObjGetDefs(vm, flags, def, persistentDef)  0)
+goto endjob;
+
+if (def) {
+if (!cfg-privileged) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(NUMA tuning is not available in session mode));
+goto endjob;
+}
+
+if (!virCgroupHasController(priv-cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(cgroup cpuset controller is not mounted));
+goto endjob;
+}
+
 if (mode != -1 
-virDomainNumatuneGetMode(vm-def-numa, -1, config_mode) == 0 
+virDomainNumatuneGetMode(def-numa, -1, config_mode) == 0 
 config_mode != mode) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(can't change numatune mode for running domain));
@@ -10225,8 +10218,8 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 qemuDomainSetNumaParamsLive(vm, nodeset)  0)
 goto endjob;

-if (virDomainNumatuneSet(vm-def-numa,
- vm-def-placement_mode ==
+if (virDomainNumatuneSet(def-numa,
+ def-placement_mode ==
  VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
  -1, mode, nodeset)  0)
 goto endjob;
@@ -10235,7 +10228,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 goto endjob;
 }

-if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
+if (persistentDef) {
 if (virDomainNumatuneSet(persistentDef-numa,
  persistentDef-placement_mode ==
  VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
@@ -10254,7 +10247,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  cleanup:
 virBitmapFree(nodeset);
 virDomainObjEndAPI(vm);
-virObjectUnref(caps);
 virObjectUnref(cfg);
 return ret;
 }

[libvirt] [PATCH 07/13] qemu: Simplify qemuDomainGetVcpusFlags by using virDomainObjGetOneDef

2015-06-15 Thread Peter Krempa
virDomainObjGetOneDef is simpler to use than virDomainObjGetDefs
---
 src/qemu/qemu_driver.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c878409..2cb0215 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5479,7 +5479,6 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 qemuDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 virDomainDefPtr def;
-virDomainDefPtr persistentDef;
 int ret = -1;
 qemuAgentCPUInfoPtr cpuinfo = NULL;
 int ncpuinfo = -1;
@@ -5498,11 +5497,11 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 if (virDomainGetVcpusFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;

-if (virDomainObjGetDefs(vm, flags, def, persistentDef)  0)
+if (!(def = virDomainObjGetOneDef(vm, flags)))
 goto cleanup;

 if (flags  VIR_DOMAIN_VCPU_GUEST) {
-if (persistentDef) {
+if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_INVALID_ARG, %s,
_(vCPU count provided by the guest agent can only 
be 
   requested for live domains));
@@ -5543,9 +5542,6 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 ret++;
 }
 } else {
-if (!def)
-def = persistentDef;
-
 if (flags  VIR_DOMAIN_VCPU_MAXIMUM)
 ret = def-maxvcpus;
 else
-- 
2.4.1

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


[libvirt] [PATCH 05/13] qemu: Simplify qemuDomainGetVcpuPinInfo by using virDomainObjGetOneDef

2015-06-15 Thread Peter Krempa
virDomainObjGetOneDef is simpler to use than virDomainObjGetDefs
---
 src/qemu/qemu_driver.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e8b2be3..f3c53f5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5213,7 +5213,6 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
 {
 virDomainObjPtr vm = NULL;
 virDomainDefPtr def;
-virDomainDefPtr targetDef;
 int ret = -1;
 int hostcpus, vcpu;
 virBitmapPtr allcpumap = NULL;
@@ -5227,12 +5226,9 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
 if (virDomainGetVcpuPinInfoEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;

-if (virDomainObjGetDefs(vm, flags, def, targetDef)  0)
+if (!(def = virDomainObjGetOneDef(vm, flags)))
 goto cleanup;

-if (def)
-targetDef = def;
-
 if ((hostcpus = nodeGetCPUCount())  0)
 goto cleanup;

@@ -5242,8 +5238,8 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
 virBitmapSetAll(allcpumap);

 /* Clamp to actual number of vcpus */
-if (ncpumaps  targetDef-vcpus)
-ncpumaps = targetDef-vcpus;
+if (ncpumaps  def-vcpus)
+ncpumaps = def-vcpus;

 if (ncpumaps  1)
 goto cleanup;
@@ -5252,8 +5248,8 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
 virDomainPinDefPtr pininfo;
 virBitmapPtr bitmap = NULL;

-pininfo = virDomainPinFind(targetDef-cputune.vcpupin,
-   targetDef-cputune.nvcpupin,
+pininfo = virDomainPinFind(def-cputune.vcpupin,
+   def-cputune.nvcpupin,
vcpu);

 if (pininfo  pininfo-cpumask)
-- 
2.4.1

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


[libvirt] [PATCH] libxl: rework reference counting

2015-06-15 Thread Jim Fehlig
Similar to commit 540c339a for the QEMU driver, rework reference
counting in the libxl driver to make it more deterministic and
the code a bit cleaner.

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

I've been testing this patch on and off for a few weeks now using
libvirt-tck domain tests, local test scripts, and some manual tests
for good measure.  I sent the patch to Anthony Perard (cc'd) nearly
two weeks ago for testing in his OpenStack+Xen+libvirt CI loop,
although I haven't received any feedback thus far.  Also included
Martin in the cc since he encouraged this patch

https://www.redhat.com/archives/libvir-list/2015-April/msg00014.html

 src/libxl/libxl_domain.c|  32 ++
 src/libxl/libxl_domain.h|   5 +-
 src/libxl/libxl_driver.c| 274 ++--
 src/libxl/libxl_migration.c |   6 +-
 4 files changed, 128 insertions(+), 189 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 0652270..ce188ee 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -96,13 +96,12 @@ libxlDomainObjFreeJob(libxlDomainObjPrivatePtr priv)
 #define LIBXL_JOB_WAIT_TIME (1000ull * 30)
 
 /*
- * obj must be locked before calling, libxlDriverPrivatePtr must NOT be locked
+ * obj must be locked before calling
  *
  * This must be called by anything that will change the VM state
- * in any way
+ * in any way.
  *
- * Upon successful return, the object will have its ref count increased,
- * successful calls must be followed by EndJob eventually
+ * Successful calls must eventually result in a call to EndJob.
  */
 int
 libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED,
@@ -117,8 +116,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver 
ATTRIBUTE_UNUSED,
 return -1;
 then = now + LIBXL_JOB_WAIT_TIME;
 
-virObjectRef(obj);
-
 while (priv-job.active) {
 VIR_DEBUG(Wait normal job condition for starting job: %s,
   libxlDomainJobTypeToString(job));
@@ -149,21 +146,16 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver 
ATTRIBUTE_UNUSED,
 virReportSystemError(errno,
  %s, _(cannot acquire job mutex));
 
-virObjectUnref(obj);
 return -1;
 }
 
 /*
- * obj must be locked before calling
+ * obj must be locked and have a reference before calling
  *
  * To be called after completing the work associated with the
  * earlier libxlDomainBeginJob() call
- *
- * Returns true if the remaining reference count on obj is
- * non-zero, false if the reference count has dropped to zero
- * and obj is disposed.
  */
-bool
+void
 libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED,
  virDomainObjPtr obj)
 {
@@ -175,8 +167,6 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver 
ATTRIBUTE_UNUSED,
 
 libxlDomainObjResetJob(priv);
 virCondSignal(priv-job.cond);
-
-return virObjectUnref(obj);
 }
 
 static void *
@@ -485,12 +475,11 @@ libxlDomainShutdownThread(void *opaque)
 }
 
  endjob:
-if (!libxlDomainObjEndJob(driver, vm))
-vm = NULL;
+libxlDomainObjEndJob(driver, vm);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
+virObjectUnref(vm);
 if (dom_event)
 libxlDomainEventQueue(driver, dom_event);
 libxl_event_free(cfg-ctx, ev);
@@ -528,6 +517,7 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST 
libxl_event *event)
 VIR_INFO(Received event for unknown domain ID %d, event-domid);
 goto error;
 }
+virObjectRef(vm);
 
 /*
  * Start a thread to handle shutdown.  We don't want to be tying up
@@ -558,8 +548,10 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST 
libxl_event *event)
 /* Cast away any const */
 libxl_event_free(cfg-ctx, (libxl_event *)event);
 virObjectUnref(cfg);
-if (vm)
+if (vm) {
 virObjectUnlock(vm);
+virObjectUnref(vm);
+}
 VIR_FREE(shutdown_info);
 }
 
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 8c73cc4..714ed91 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -83,10 +83,9 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver,
enum libxlDomainJob job)
 ATTRIBUTE_RETURN_CHECK;
 
-bool
+void
 libxlDomainObjEndJob(libxlDriverPrivatePtr driver,
- virDomainObjPtr obj)
-ATTRIBUTE_RETURN_CHECK;
+ virDomainObjPtr obj);
 
 void
 libxlDomainEventQueue(libxlDriverPrivatePtr driver,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a7be745..c0061b3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -282,7 +282,7 @@ libxlDomObjFromDomain(virDomainPtr dom)
 libxlDriverPrivatePtr driver = dom-conn-privateData;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
+vm = 

[libvirt] [PATCH v2 3/3] storage: Generate correct parameters for CIFS

2015-06-15 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1186969

When generating the path to the dir for a CIFS/Samba driver, the code
would generate a source path for the mount using %s:%s while the
mount.cifs expects to see //%s/%s. So check for the cifsfs and
format the source path appropriately.

Additionally, since there is no means to authenticate, the mount
needs a -o guest on the command line in order to anonymously mount
the Samba directory.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatstorage.html.in   |  7 +--
 docs/storage.html.in |  3 ++-
 src/storage/storage_backend_fs.c | 27 ++-
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 17558f8..b6f4361 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -124,11 +124,14 @@
 span class=sinceSince 0.4.1/span/dd
   dtcodedir/code/dt
   ddProvides the source for pools backed by directories (pool
-type codedir/code), or optionally to select a subdirectory
+types codedir/code, codenetfs/code, codegluster/code),
+or optionally to select a subdirectory
 within a pool that resembles a filesystem (pool
 type codegluster/code). May
 only occur once. Contains a single attribute codepath/code
-which is the fully qualified path to the backing directory.
+which is the fully qualified path to the backing directory or
+for a codenetfs/code pool type using codeformat/code
+type cifs, the path to the Samba share without the leading slash.
 span class=sinceSince 0.4.1/span/dd
   dtcodeadapter/code/dt
   ddProvides the source for pools backed by SCSI adapters (pool
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 92e9ae7..0b467d5 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -291,7 +291,8 @@
 the a href=#StorageBackendGlustergluster/a pool.)
   /li
   li
-codecifs/code - use the SMB (samba) or CIFS file system
+codecifs/code - use the SMB (samba) or CIFS file system.
+The mount will use -o guest to mount the directory anonymously.
   /li
 /ul
 
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index d2cf470..b751687 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -426,6 +426,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
 pool-def-source.format == VIR_STORAGE_POOL_NETFS_AUTO);
 bool glusterfs = (pool-def-type == VIR_STORAGE_POOL_NETFS 
   pool-def-source.format == 
VIR_STORAGE_POOL_NETFS_GLUSTERFS);
+bool cifsfs = (pool-def-type == VIR_STORAGE_POOL_NETFS 
+   pool-def-source.format == VIR_STORAGE_POOL_NETFS_CIFS);
 virCommandPtr cmd = NULL;
 int ret = -1;
 int rc;
@@ -444,11 +446,17 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr 
pool)
 }
 
 if (pool-def-type == VIR_STORAGE_POOL_NETFS) {
-if (virAsprintf(src, %s:%s,
-pool-def-source.hosts[0].name,
-pool-def-source.dir) == -1)
-return -1;
-
+if (pool-def-source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
+if (virAsprintf(src, //%s/%s,
+pool-def-source.hosts[0].name,
+pool-def-source.dir) == -1)
+return -1;
+} else {
+if (virAsprintf(src, %s:%s,
+pool-def-source.hosts[0].name,
+pool-def-source.dir) == -1)
+return -1;
+}
 } else {
 if (VIR_STRDUP(src, pool-def-source.devices[0].path)  0)
 return -1;
@@ -468,6 +476,15 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
direct-io-mode=1,
pool-def-target.path,
NULL);
+else if (cifsfs)
+cmd = virCommandNewArgList(MOUNT,
+   -t,
+   
virStoragePoolFormatFileSystemNetTypeToString(pool-def-source.format),
+   src,
+   pool-def-target.path,
+   -o,
+   guest,
+   NULL);
 else
 cmd = virCommandNewArgList(MOUNT,
-t,
-- 
2.1.0

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


[libvirt] [PATCH v2 0/3] Adjust netfs CIFS/Samba formatting

2015-06-15 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1186969

v1 here:
http://www.redhat.com/archives/libvir-list/2015-June/msg00155.html

Changes since v1:

 * Alter v1 to make a common RNG - even though it's ACK'd - I figured I'd
   still send to make sure there were no other issues...

 * Add 2/3 to apply comment made in former 2/2 regarding checking for
   pool-def-type == VIR_STORAGE_POOL_FS to the glusterfs since it has
   the same issue

 * Modify 3/3 is old 2/2 and only removed the ternary check.

John Ferlan (3):
  storage: Fix the schema and add tests for cifs pool
  storage: Adjust command arglist for gluster
  storage: Generate correct parameters for CIFS

 docs/formatstorage.html.in  |  7 --
 docs/schemas/storagepool.rng| 10 
 docs/storage.html.in|  3 ++-
 src/storage/storage_backend_fs.c| 31 ++---
 tests/storagepoolxml2xmlin/pool-netfs-cifs.xml  | 12 ++
 tests/storagepoolxml2xmlout/pool-netfs-cifs.xml | 15 
 tests/storagepoolxml2xmltest.c  |  1 +
 7 files changed, 64 insertions(+), 15 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-cifs.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-cifs.xml

-- 
2.1.0

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


[libvirt] [PATCH v2 1/3] storage: Fix the schema and add tests for cifs pool

2015-06-15 Thread John Ferlan
Commit id '887dd362' added support for a netfs pool format type 'cifs'
and 'gluster' in order to add rng support for Samba and glusterfs netfs
pools. Originally, the CIFS type support was added as part of commit
id '61fb6979'. Eventually commit id 'b325be12' fixed the gluster rng
definition to match expectations.

As it turns out the CIFS rng needed a similar change since the directory
path is not an absDirPath, rather just a dirPath will be required.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/schemas/storagepool.rng| 10 ++
 tests/storagepoolxml2xmlin/pool-netfs-cifs.xml  | 12 
 tests/storagepoolxml2xmlout/pool-netfs-cifs.xml | 15 +++
 tests/storagepoolxml2xmltest.c  |  1 +
 4 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-cifs.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-cifs.xml

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index db6ff49..9c52817 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -314,7 +314,7 @@
 /element
   /define
 
-  define name='sourceinfonetfsgluster'
+  define name='sourceinfonetrelativepath'
 element name='dir'
   attribute name='path'
 ref name='dirPath'/
@@ -400,7 +400,6 @@
   choice
 valueauto/value
 valuenfs/value
-valuecifs/value
   /choice
 /attribute
   /element
@@ -488,10 +487,13 @@
 group
   interleave
 ref name='sourceinfohost'/
-ref name='sourceinfonetfsgluster'/
+ref name='sourceinfonetrelativepath'/
 element name='format'
   attribute name='type'
-valueglusterfs/value
+choice
+  valuecifs/value
+  valueglusterfs/value
+/choice
   /attribute
 /element
 optional
diff --git a/tests/storagepoolxml2xmlin/pool-netfs-cifs.xml 
b/tests/storagepoolxml2xmlin/pool-netfs-cifs.xml
new file mode 100644
index 000..0bc6380
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-netfs-cifs.xml
@@ -0,0 +1,12 @@
+pool type='netfs'
+  source
+host name='example.com'/
+format type='cifs'/
+dir path='samba_share'/
+  /source
+  namenetfs-cifs/name
+  uuidd5609ced-94b1-489e-b218-eff35c30336a/uuid
+  target
+path/mnt/cifs/path
+  /target
+/pool
diff --git a/tests/storagepoolxml2xmlout/pool-netfs-cifs.xml 
b/tests/storagepoolxml2xmlout/pool-netfs-cifs.xml
new file mode 100644
index 000..afaa7d0
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-netfs-cifs.xml
@@ -0,0 +1,15 @@
+pool type='netfs'
+  namenetfs-cifs/name
+  uuidd5609ced-94b1-489e-b218-eff35c30336a/uuid
+  capacity unit='bytes'0/capacity
+  allocation unit='bytes'0/allocation
+  available unit='bytes'0/available
+  source
+host name='example.com'/
+dir path='samba_share'/
+format type='cifs'/
+  /source
+  target
+path/mnt/cifs/path
+  /target
+/pool
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index bec1b8f..b03c4b0 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -84,6 +84,7 @@ mymain(void)
 DO_TEST(pool-iscsi-auth);
 DO_TEST(pool-netfs);
 DO_TEST(pool-netfs-gluster);
+DO_TEST(pool-netfs-cifs);
 DO_TEST(pool-scsi);
 DO_TEST(pool-scsi-type-scsi-host);
 DO_TEST(pool-scsi-type-fc-host);
-- 
2.1.0

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


Re: [libvirt] [PATCH] Fix virsh edit and virt-xml-validate with SCSI LUN

2015-06-15 Thread John Ferlan

Something I didn't catch on pass1, but perhaps an important distinction

title:

Resolve issues with SCSI LUN hostdev device addressing


Coincidentally - there's a bz regarding the address setting between
SCSI disk and hostdev which is assigned to me and was near the top
of my todo list... https://bugzilla.redhat.com/show_bug.cgi?id=1210587
so this has helped put me in the right frame of reference!

...

 Need to add some XML in order to test the longer values - eg, xml2xml
 and/or xml2args in order to validate that what you read in is what gets
 printed out and that what you read in gets sent to the hypervisor
 correctly.
 
 I wrestled with this part, because the xml2args tests pass the SCSI
 [host:bus:target:unit] address to testSCSIDeviceGetSgName, which ignores
 all inputs and returns /dev/sg0.  The reverse had a similar gotcha. 
 I'll dig at xml2xml, and see what I can come up with.
 
me wonders what qemu dos with a 20 digit unit number...

 So speaking of qemu... If you look at qemuBuildDriveStr you will see how
 the device address is passed down to qemu... That function needs some
 adjustment too it seems.
 
 This function doesn't appear to be invoked for an address tag within a
 hostdev element, either as part of the source subelements or as the
 address being created for the guest.  Rather, it seems to turn up as
 part of a disk element, which (unless I'm mistaken) relies on the host
 /dev address instead of [host:bus:target:unit].

Ah... so back to my title comment ;-) - hostdev...  That would mean
qemuBuildSCSIHostdevDevStr, which does have a printing of bus, target,
unit.  Still wondering about how qemu would handle it (haven't got that
far yet), but that seems to be what you point out in the next paragraph.

 
 Speaking of the address that is created in the guest, though, I left
 that as-is because virtio defines LUN addresses up to 16,384 and is
 enforced in KVM/QEMU.  Since I can't create a unit address in the guest
 larger than that, leaving that defined as an int is okay.
 

And so regardless of what you did for libvirt, kvm/qemu wouldn't be able
to handle it?  Perhaps then that needs to be checked somehow... Is this
something qemu/kvm needs to support?  Again I haven't looked at those
sources yet.  Is there a specific hypervisor that isn't working because
libvirt isn't passing the correct address value?  This is the what is
the bug you're trying to fix type question...


 I didn't dig into the qemu sources yet, but


Looks like I also lost my train of thought here ;-)

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 1781996..e7a8e1a 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -2827,7 +2827,7 @@
 ddDrive addresses have the following additional
   attributes: codecontroller/code (a 2-digit controller
   number), codebus/code (a 2-digit bus number),
 -codetarget/code (a 2-digit bus number),
 +codetarget/code (a 2-digit target number),
   and codeunit/code (a 2-digit unit number on the bus).
 /dd
 dtcodetype='virtio-serial'/code/dt
 Interesting there's no 'scsi' in here (of course you're removing it
 below, but that leaves the address as unknown
 
 Is it?  See below, but I thought that got hardcoded somewhere because
 it's in a hostdev.  I'll doublecheck.
 

It seems virDomainHostdevSubsysSCSIHostDefParseXML ignores the type
field and the RNG supports that.



 @@ -3136,7 +3136,7 @@
   lt;hostdev mode='subsystem' type='scsi' sgio='filtered'
 rawio='yes'gt;
 lt;sourcegt;
   lt;adapter name='scsi_host0'/gt;
 -lt;address type='scsi' bus='0' target='0' unit='0'/gt;
 +lt;address bus='0' target='0' unit='0'/gt;
 These first two don't seem relevant to the changes below and should have
 been their own patch.
 
 Fair enough, my apologies.  I'll break them out.
 

We all fall into the same trap at some point in time ;-)


 See [1] below for a side comment on this particular changes



...

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 36de844..1f2bfca 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -4954,7 +4954,7 @@
 virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
   goto cleanup;
   }
   -if (virStrToLong_ui(unit, NULL, 0,
 scsihostsrc-unit)  0) {
 +if (virStrToLong_ull(unit, NULL, 0,
 scsihostsrc-unit)  0) {
   virReportError(VIR_ERR_INTERNAL_ERROR,
  _(cannot parse unit '%s'), unit);
 Existing - read as ui (or now ull), but could be uip and ullp
 (eg, don't allow reading a negative value).

 I would be remiss if I didn't point out UINT_MAX is 4294967295U, which
 is 10 digits, but means 99 which would be 'valid' according to
 the 10 digit rule in RNG but invalid once you read it in...

   goto cleanup;
 @@ -18844,7 

[libvirt] [PATCH v2 2/3] storage: Adjust command arglist for gluster

2015-06-15 Thread John Ferlan
In order for the glusterfs boolean to be set, the pool-def-type must be
VIR_STORAGE_POOL_NETFS, thus the check within virCommandNewArgList whether
pool-def-type is VIR_STORAGE_POOL_FS will never be true, so remove it

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/storage/storage_backend_fs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 8487f08..d2cf470 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -462,9 +462,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
 else if (glusterfs)
 cmd = virCommandNewArgList(MOUNT,
-t,
-   (pool-def-type == VIR_STORAGE_POOL_FS ?
-
virStoragePoolFormatFileSystemTypeToString(pool-def-source.format) :
-
virStoragePoolFormatFileSystemNetTypeToString(pool-def-source.format)),
+   
virStoragePoolFormatFileSystemNetTypeToString(pool-def-source.format),
src,
-o,
direct-io-mode=1,
-- 
2.1.0

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


[libvirt] Libvirt bite sized tasks

2015-06-15 Thread Michal Privoznik
Dear lists,

I've just started new wiki page which aim is to summarize small and
trivial tasks, that starting contributors can take, investigate and
implement. The aim is to give them something easy to start with while
not scaring them out about complexity of our code. The page can be found
here:

http://wiki.libvirt.org/page/BiteSizedTasks

The name is copied from qemu wiki:

http://wiki.qemu.org/BiteSizedTasks

Please, feel free to extend the list. I plan to use it in the future
when interviewing GSoC candidates.

Michal

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


Re: [libvirt] [PATCH] qemu: fix cannot attach a virtio channel

2015-06-15 Thread Ján Tomko
I'd rather be more specific in the commit summary, e.g.:
fix address allocation on chardev hotplug

On Wed, Jun 10, 2015 at 10:49:37PM +0800, Luyao Huang wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1230039
 
 When attach a channel which target type is virtio, we will
 get an error:
 
 error: Failed to attach device from channel-file.xml
 error: internal error: virtio serial device has invalid address type
 
 This issue was introduced in commit 9807c4.
 We didn't check the chr device type is serial then check
 if the device target type is pci-serial, but not all the
 Chr device is a serial type so we should check the device
 type before check the target type to avoid assign wrong
 address to other device type chr device.
 
 Also most of chr device do not need {pci, virtio-serial} address
 in qemu, we just get the address for the device which needed.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  src/qemu/qemu_hotplug.c | 72 
 +++--
  1 file changed, 46 insertions(+), 26 deletions(-)
 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 3562de6..4d60513 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1531,6 +1531,48 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
  return ret;
  }
  
 +static int
 +qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
 +virDomainChrDefPtr chr)
 +{
 +if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
 +chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO 
 +virDomainVirtioSerialAddrAutoAssign(NULL, priv-vioserialaddrs,
 +chr-info, true)  0)
 +return -1;
 +
 +if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL 
 +chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI 

 +(chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
 + chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) 

Do we need to check the address type here? pci-serial should always have
a PCI address.

 +virDomainPCIAddressEnsureAddr(priv-pciaddrs, chr-info)  0)
 +return -1;
 +
 +if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL 
 +chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO 
 +virDomainVirtioSerialAddrAutoAssign(NULL, priv-vioserialaddrs,
 +chr-info, false)  0)
 +return -1;
 +
 +return 0;
 +}
 +
 +static void
 +qemuDomainAttachChrDeviceReleaseAddr(qemuDomainObjPrivatePtr priv,
 + virDomainObjPtr vm,
 + virDomainChrDefPtr chr)
 +{
 +if ((chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
 + chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) ||
 +(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL 
 + chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))
 +virDomainVirtioSerialAddrRelease(priv-vioserialaddrs, chr-info);
 +
 +if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL 
 +chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI)
 +qemuDomainReleaseDeviceAddress(vm, chr-info, NULL);
 +}
 +
  int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr)
 @@ -1541,7 +1583,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  char *devstr = NULL;
  char *charAlias = NULL;
  bool need_release = false;
 -bool allowZero = false;
  
  if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
  virReportError(VIR_ERR_OPERATION_INVALID, %s,
 @@ -1552,22 +1593,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  if (qemuAssignDeviceChrAlias(vmdef, chr, -1)  0)
  goto cleanup;
  
 -if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
 -chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
 -allowZero = true;
 -
 -if (chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
 -if (virDomainPCIAddressEnsureAddr(priv-pciaddrs, chr-info)  0)
 -goto cleanup;
 -} else if (chr-targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
 -/* XXX */
 -} else {
 -if (virDomainVirtioSerialAddrAutoAssign(NULL,
 -priv-vioserialaddrs,
 -chr-info,
 -allowZero)  0)
 -goto cleanup;
 -}
 +if (qemuDomainAttachChrDeviceAssignAddr(priv, chr)  0)
 +goto cleanup;
  need_release = true;
  
  if (qemuBuildChrDeviceStr(devstr, vm-def, chr, priv-qemuCaps)  0)
 @@ -1601,15 +1628,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
   cleanup:
  if (ret  0  virDomainObjIsActive(vm))
  

Re: [libvirt] [PATCH] virCapabilitiesDomainDataLookup: Produce saner error message

2015-06-15 Thread Erik Skultety


On 06/12/2015 03:49 PM, Michal Privoznik wrote:
 During a review, I've noticed this error message that was eventually
 produced when I was trying to define a domain:
 
 error: invalid argument: could not find capabilities for arch=mips64el
 domaintype=(null)
 
 Look at the (null). Why is it there? Well, during XML parsing, we try
 to look up the default emulator for given OS type and possibly virt
 type too. And this is the problem, because if we don't want to look up
 by virt type, a -1 is passed to note this fact. Later, the code
 handles -1 just right. Except for error message. When it is
 constructed (in a very fabulous way I must say), the value is compared
 to zero, not -1. And since we don't have any translation from -1 to a
 virt type string, we just print (null).
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/capabilities.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
 index 6decde8..9c2c6b4 100644
 --- a/src/conf/capabilities.c
 +++ b/src/conf/capabilities.c
 @@ -678,7 +678,7 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
virDomainOSTypeToString(ostype));
  if (arch)
  virBufferAsprintf(buf, arch=%s , virArchToString(arch));
 -if (domaintype)
 +if (domaintype != -1)
Well, this will work. However for some reason I don't quite like this
way of handling it. We're trying to construct an error message and
because we didn't want to search by virttype we special-case this. I was
thinking more about introducing a new enum VIR_DOMAIN_VIRT_NONE which
will serve exactly this purpose (see virCapsDomainDataCompare,
comparison to arch :)). That way, we don't have to play with ternary in
here as pass VIRT_NONE right away:
...if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def-os.type,
def-os.arch, use_virttype ? def-virtType : -1,
NULL, NULL)))...
Since XML parsing requires a domain to have a valid virttype defined, we
should be able to abuse the VIRT_NONE this way and stay a little
consistent in this particular code snippet of constructing an error
message. What do you think about it? If you think it's not worth it at
all, just drop the idea and push this one, it's an ACK.
  virBufferAsprintf(buf, domaintype=%s ,
virDomainVirtTypeToString(domaintype));
  if (emulator)
 

Erik

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


Re: [libvirt] Deleted all setting after reboot

2015-06-15 Thread Michal Privoznik
On 13.06.2015 07:48, Amir Azemati wrote:
 Hello all great,
 
 We had a question, We use OpenStack IceHouse on CentOS 6.5.
 We use libvirt for create network, attach interface to our VMs and ...
 
 There is a problem. When our server restart (reboot), all settings are
 deleted. We have to go back and do all the setting (create network, attach
 an interface and ...) from the beginning.
 
 How can we solve our problem.
 
 our command we enterd:
 
 virsh list
 virsh domiflist ID
 virsh attach-interface --domain ID --type bridge --source br100
 virsh managedsave ID

There are two levels of operation here. One is over so called live XML,
the other one is config XML. Anything that's in live gets forgotten not
only on host reboot but on domain shutdown  start cycle. If I were to
reminiscent it to something, then it would be firewalld. Creating a live
rule works, only until the time firewall is reloaded.
So, in order to make your changes persistent you should:

1) define the network. This way libvirt will remember it even during
reboots. 'virsh net-create' is again something temporary.

2) make the network autostart: 'virsh net-autostart $net'

3) Attach the interface on config level too: 'virsh attach-interface
--config ...'

Michal

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


Re: [libvirt] [PATCH] virCapabilitiesDomainDataLookup: Produce saner error message

2015-06-15 Thread Michal Privoznik
On 15.06.2015 09:10, Erik Skultety wrote:
 
 
 On 06/12/2015 03:49 PM, Michal Privoznik wrote:
 During a review, I've noticed this error message that was eventually
 produced when I was trying to define a domain:

 error: invalid argument: could not find capabilities for arch=mips64el
 domaintype=(null)

 Look at the (null). Why is it there? Well, during XML parsing, we try
 to look up the default emulator for given OS type and possibly virt
 type too. And this is the problem, because if we don't want to look up
 by virt type, a -1 is passed to note this fact. Later, the code
 handles -1 just right. Except for error message. When it is
 constructed (in a very fabulous way I must say), the value is compared
 to zero, not -1. And since we don't have any translation from -1 to a
 virt type string, we just print (null).

 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/capabilities.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
 index 6decde8..9c2c6b4 100644
 --- a/src/conf/capabilities.c
 +++ b/src/conf/capabilities.c
 @@ -678,7 +678,7 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
virDomainOSTypeToString(ostype));
  if (arch)
  virBufferAsprintf(buf, arch=%s , virArchToString(arch));
 -if (domaintype)
 +if (domaintype != -1)
 Well, this will work. However for some reason I don't quite like this
 way of handling it. We're trying to construct an error message and
 because we didn't want to search by virttype we special-case this. I was
 thinking more about introducing a new enum VIR_DOMAIN_VIRT_NONE which
 will serve exactly this purpose (see virCapsDomainDataCompare,
 comparison to arch :)). That way, we don't have to play with ternary in
 here as pass VIRT_NONE right away:
 ...if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def-os.type,
 def-os.arch, use_virttype ? def-virtType : -1,
 NULL, NULL)))...
 Since XML parsing requires a domain to have a valid virttype defined, we
 should be able to abuse the VIRT_NONE this way and stay a little
 consistent in this particular code snippet of constructing an error
 message. What do you think about it? If you think it's not worth it at
 all, just drop the idea and push this one, it's an ACK.

I think it is worth it. However, it would require slightly bigger change
since the rest of lookup functions expects -1 as an alias for 'not
specified'. However, I've started new wiki page (after example given by
qemu):

  http://wiki.libvirt.org/page/BiteSizedTasks

and put your idea there. Feel free to reword and adjust it. Maybe I
should send out more explicit e-mail to let others know about the page
too. Anyway, I've pushed this as-is. Thanks!

Michal

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


[libvirt] [PATCH v4 1/9] virDomainDiskGetSource: Mark passed disk as 'const'

2015-06-15 Thread Pavel Boldin
From: Michal Privoznik mpriv...@redhat.com

The disk is not changed anywhere in the function. Mark this fact
in the function header too.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c | 2 +-
 src/conf/domain_conf.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 36de844..1f433ee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1467,7 +1467,7 @@ virDomainDiskSetType(virDomainDiskDefPtr def, int type)
 
 
 const char *
-virDomainDiskGetSource(virDomainDiskDefPtr def)
+virDomainDiskGetSource(virDomainDiskDef const *def)
 {
 return def-src-path;
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba17a8d..bddf837 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2450,7 +2450,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def);
 void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
 int virDomainDiskGetType(virDomainDiskDefPtr def);
 void virDomainDiskSetType(virDomainDiskDefPtr def, int type);
-const char *virDomainDiskGetSource(virDomainDiskDefPtr def);
+const char *virDomainDiskGetSource(virDomainDiskDef const *def);
 int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
 ATTRIBUTE_RETURN_CHECK;
 const char *virDomainDiskGetDriver(virDomainDiskDefPtr def);
-- 
1.9.1

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


[libvirt] [PATCH v4 5/9] util: multi-value parameters in virTypedParamsAdd*

2015-06-15 Thread Pavel Boldin
Allow multi-value parameters to be build using virTypedParamsAdd*
functions by removing check for duplicates.

Signed-off-by: Pavel Boldin pbol...@mirantis.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/util/virtypedparam.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 68620f5..6f608d6 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -749,14 +749,6 @@ virTypedParamsGetString(virTypedParameterPtr params,
 }
 
 
-#define VIR_TYPED_PARAM_CHECK() \
-do { if (virTypedParamsGet(*params, n, name)) { \
-virReportError(VIR_ERR_INVALID_ARG, \
-   _(Parameter '%s' is already set), name);   \
-goto error; \
-} } while (0)
-
-
 /**
  * virTypedParamsAddInt:
  * @params: pointer to the array of typed parameters
@@ -787,7 +779,6 @@ virTypedParamsAddInt(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -835,7 +826,6 @@ virTypedParamsAddUInt(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -883,7 +873,6 @@ virTypedParamsAddLLong(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -931,7 +920,6 @@ virTypedParamsAddULLong(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -979,7 +967,6 @@ virTypedParamsAddDouble(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -1027,7 +1014,6 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -1078,7 +1064,6 @@ virTypedParamsAddString(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
@@ -1136,7 +1121,6 @@ virTypedParamsAddFromString(virTypedParameterPtr *params,
 
 virResetLastError();
 
-VIR_TYPED_PARAM_CHECK();
 if (VIR_RESIZE_N(*params, max, n, 1)  0)
 goto error;
 *maxparams = max;
-- 
1.9.1

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


[libvirt] [PATCH v4 0/9] Selective block device migration implementation

2015-06-15 Thread Pavel Boldin
Behold of the fourth re-roll of the selective block device migration patch.
In this patch we don't only fix the issue with NBD migration format auto-
detection but also introduce the patches that do enhance the NBD migration
triggered by `drive-mirror` QEMU command with ability for the user to select
what disks are to be migrated based on the target name.

First two patches fix some nitpicks, third one fixes the issue with NBD format
auto-detection.

Middle ones introduce a necessary API to keep a list of block devices to
migrate in the virTypedParameter array and to work with this list.

Of the two last patches first introduces the `migrate_disks' qemuMigration*
parameter and pushes it down the call stack making the code to consult it when
there is a decision to be made whether the block device is to be migrated to
the new host. When there is no `migrate_disks' parameter given then the old
scheme is used: only non-shared non-readonly disks with a source are migrated.

The last patch promotes this ability up to the virsh utility and documents
it as appropriate.

Michal Privoznik (3):
  virDomainDiskGetSource: Mark passed disk as 'const'
  qemuMigrationBeginPhase: Fix function header indentation
  qemuMigrationDriveMirror: Force raw format for NBD

Pavel Boldin (6):
  util: multi-value virTypedParameter
  util: multi-value parameters in virTypedParamsAdd*
  util: virTypedParams{Filter,GetAllStrings}
  util: add virTypedParamsAddStringList
  qemu: migration: selective block device migration
  virsh: selective block device migration

 include/libvirt/libvirt-domain.h |   9 ++
 include/libvirt/libvirt-host.h   |  11 ++
 src/conf/domain_conf.c   |   2 +-
 src/conf/domain_conf.h   |   2 +-
 src/libvirt_public.syms  |   6 +
 src/qemu/qemu_driver.c   |  78 ---
 src/qemu/qemu_migration.c| 264 +--
 src/qemu/qemu_migration.h|  24 ++--
 src/util/virtypedparam.c | 259 +++---
 src/util/virtypedparam.h |  19 +++
 tests/Makefile.am|   6 +
 tests/virtypedparamtest.c| 295 +++
 tools/virsh-domain.c |  23 +++
 tools/virsh.pod  |  21 +--
 14 files changed, 854 insertions(+), 165 deletions(-)
 create mode 100644 tests/virtypedparamtest.c

-- 
1.9.1

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


[libvirt] [PATCH v4 8/9] qemu: migration: selective block device migration

2015-06-15 Thread Pavel Boldin
Implement a `migrate_disks' parameters for the QEMU driver. This multi-
value parameter can be used to explicitly specify what block devices
are to be migrated using the NBD server. Tunnelled migration using NBD
is to be done.

Signed-off-by: Pavel Boldin pbol...@mirantis.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 include/libvirt/libvirt-domain.h |   9 ++
 src/qemu/qemu_driver.c   |  78 ++---
 src/qemu/qemu_migration.c| 245 ---
 src/qemu/qemu_migration.h|  24 ++--
 4 files changed, 264 insertions(+), 92 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index d851225..7564c20 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -748,6 +748,15 @@ typedef enum {
  */
 # define VIR_MIGRATE_PARAM_LISTEN_ADDRESSlisten_address
 
+/**
+ * VIR_MIGRATE_PARAM_MIGRATE_DISKS:
+ *
+ * virDomainMigrate* params multiple field: The multiple values that list
+ * the block devices to be migrated. At the moment this is only supported
+ * by the QEMU driver but not for the tunnelled migration.
+ */
+# define VIR_MIGRATE_PARAM_MIGRATE_DISKSmigrate_disks
+
 /* Domain migration. */
 virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn,
unsigned long flags, const char *dname,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 34e5581..c244f46 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12226,7 +12226,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
 ret = qemuMigrationPrepareDirect(driver, dconn,
  NULL, 0, NULL, NULL, /* No cookies */
  uri_in, uri_out,
- def, origname, NULL, flags);
+ def, origname, NULL, 0, NULL, flags);
 
  cleanup:
 VIR_FREE(origname);
@@ -12279,7 +12279,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
  * Consume any cookie we were able to decode though
  */
 ret = qemuMigrationPerform(driver, dom-conn, vm,
-   NULL, dconnuri, uri, NULL, NULL,
+   NULL, dconnuri, uri, NULL, NULL, 0, NULL,
cookie, cookielen,
NULL, NULL, /* No output cookies in v2 */
flags, dname, resource, false);
@@ -12356,7 +12356,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
 }
 
 return qemuMigrationBegin(domain-conn, vm, xmlin, dname,
-  cookieout, cookieoutlen, flags);
+  cookieout, cookieoutlen, flags, 0, NULL);
 }
 
 static char *
@@ -12369,11 +12369,14 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain,
 {
 const char *xmlin = NULL;
 const char *dname = NULL;
+const char **migrate_disks = NULL;
+int nmigrate_disks;
+char *ret = NULL;
 virDomainObjPtr vm;
 
 virCheckFlags(QEMU_MIGRATION_FLAGS, NULL);
 if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS)  0)
-return NULL;
+goto cleanup;
 
 if (virTypedParamsGetString(params, nparams,
 VIR_MIGRATE_PARAM_DEST_XML,
@@ -12381,18 +12384,30 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain,
 virTypedParamsGetString(params, nparams,
 VIR_MIGRATE_PARAM_DEST_NAME,
 dname)  0)
-return NULL;
+goto cleanup;
+
+nmigrate_disks = virTypedParamsGetAllStrings(
+params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS,
+migrate_disks);
+
+if (nmigrate_disks  0)
+goto cleanup;
 
 if (!(vm = qemuDomObjFromDomain(domain)))
-return NULL;
+goto cleanup;
 
 if (virDomainMigrateBegin3ParamsEnsureACL(domain-conn, vm-def)  0) {
 virDomainObjEndAPI(vm);
-return NULL;
+goto cleanup;
 }
 
-return qemuMigrationBegin(domain-conn, vm, xmlin, dname,
-  cookieout, cookieoutlen, flags);
+ret = qemuMigrationBegin(domain-conn, vm, xmlin, dname,
+ cookieout, cookieoutlen, flags,
+ nmigrate_disks, migrate_disks);
+
+ cleanup:
+VIR_FREE(migrate_disks);
+return ret;
 }
 
 
@@ -12436,7 +12451,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
  cookiein, cookieinlen,
  cookieout, cookieoutlen,
  uri_in, uri_out,
- def, origname, NULL, flags);
+ def, origname, NULL, 0, NULL, flags);
 
  cleanup:
 VIR_FREE(origname);
@@ -12462,6 +12477,8 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
 const char *dname = NULL;
 

[libvirt] [PATCH v4 6/9] util: virTypedParams{Filter, GetAllStrings}

2015-06-15 Thread Pavel Boldin
Add multikey API:

 * virTypedParamsFilter that filters all the parameters with specified name.
 * virTypedParamsGetAllStrings that returns a list with all the values for
   specified name and string type.

Signed-off-by: Pavel Boldin pbol...@mirantis.com
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 include/libvirt/libvirt-host.h |   5 ++
 src/libvirt_public.syms|   5 ++
 src/util/virtypedparam.c   | 102 +
 src/util/virtypedparam.h   |   9 
 tests/Makefile.am  |   2 +-
 tests/virtypedparamtest.c  | 100 
 6 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
index 070550b..8222cfb 100644
--- a/include/libvirt/libvirt-host.h
+++ b/include/libvirt/libvirt-host.h
@@ -284,6 +284,11 @@ virTypedParamsGetString (virTypedParameterPtr params,
  const char *name,
  const char **value);
 int
+virTypedParamsGetAllStrings(virTypedParameterPtr params,
+ int nparams,
+ const char *name,
+ const char ***values);
+int
 virTypedParamsAddInt(virTypedParameterPtr *params,
  int *nparams,
  int *maxparams,
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 716dd2f..0a1feea 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -715,4 +715,9 @@ LIBVIRT_1.2.16 {
 virDomainSetUserPassword;
 } LIBVIRT_1.2.15;
 
+LIBVIRT_1.3.0 {
+global:
+virTypedParamsGetAllStrings;
+} LIBVIRT_1.2.16;
+
 #  define new API here using predicted next version number 
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 6f608d6..a12006c 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -482,6 +482,51 @@ virTypedParamsGet(virTypedParameterPtr params,
 }
 
 
+/**
+ * virTypedParamsFilter:
+ * @params: array of typed parameters
+ * @nparams: number of parameters in the @params array
+ * @name: name of the parameter to find
+ * @ret: pointer to the returned array
+ *
+ * Filters @params retaining only the parameters named @name in the
+ * resulting array @ret. Caller should free the @ret array but not
+ * the items since they are pointing to the @params elements.
+ *
+ * Returns amount of elements in @ret on success, -1 on error.
+ */
+int
+virTypedParamsFilter(virTypedParameterPtr params,
+ int nparams,
+ const char *name,
+ virTypedParameterPtr **ret)
+{
+size_t i, alloc = 0, n = 0;
+
+virCheckNonNullArgGoto(params, error);
+virCheckNonNullArgGoto(name, error);
+virCheckNonNullArgGoto(ret, error);
+
+*ret = NULL;
+
+for (i = 0; i  nparams; i++) {
+if (STREQ(params[i].field, name)) {
+if (VIR_RESIZE_N(*ret, alloc, n, 1)  0)
+goto error;
+
+(*ret)[n] = params[i];
+
+n++;
+}
+}
+
+return n;
+
+ error:
+return -1;
+}
+
+
 #define VIR_TYPED_PARAM_CHECK_TYPE(check_type)  \
 do { if (param-type != check_type) {   \
 virReportError(VIR_ERR_INVALID_ARG, \
@@ -750,6 +795,63 @@ virTypedParamsGetString(virTypedParameterPtr params,
 
 
 /**
+ * virTypedParamsGetAllStrings:
+ * @params: array of typed parameters
+ * @nparams: number of parameters in the @params array
+ * @name: name of the parameter to find
+ * @values: array of returned values
+ *
+ * Finds all parameters with desired @name within @params and
+ * store their values into @values. The @values array is self
+ * allocated and its length is stored into @picked. When no
+ * longer needed, caller should free the returned array, but not
+ * the items since they are taken from @params array.
+ *
+ * Returns amount of strings in @values array on success,
+ * -1 otherwise.
+ */
+int
+virTypedParamsGetAllStrings(virTypedParameterPtr params,
+int nparams,
+const char *name,
+const char ***values)
+{
+size_t i, n;
+int nfiltered;
+virTypedParameterPtr *filtered = NULL;
+
+virResetLastError();
+
+virCheckNonNullArgGoto(values, error);
+*values = NULL;
+
+nfiltered = virTypedParamsFilter(params, nparams, name, filtered);
+
+if (nfiltered  0)
+goto error;
+
+if (nfiltered 
+VIR_ALLOC_N(*values, nfiltered)  0)
+goto error;
+
+for (n = 0, i = 0; i  nfiltered; i++) {
+if (filtered[i]-type == VIR_TYPED_PARAM_STRING)
+(*values)[n++] = filtered[i]-value.s;
+}
+
+VIR_FREE(filtered);
+return n;
+
+ error:
+if (values)
+VIR_FREE(*values);
+VIR_FREE(filtered);
+virDispatchError(NULL);
+return -1;

[libvirt] [PATCH v4 2/9] qemuMigrationBeginPhase: Fix function header indentation

2015-06-15 Thread Pavel Boldin
From: Michal Privoznik mpriv...@redhat.com

This function is returning a string (domain XML). Since d3ce7363
when it was first introduced, it was indented incorrectly:

static char
*qemuMigrationBeginPhase(..)

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/qemu/qemu_migration.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 70400f3..6c94052 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2699,14 +2699,14 @@ qemuMigrationCleanup(virDomainObjPtr vm,
 
 
 /* The caller is supposed to lock the vm and start a migration job. */
-static char
-*qemuMigrationBeginPhase(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- const char *xmlin,
- const char *dname,
- char **cookieout,
- int *cookieoutlen,
- unsigned long flags)
+static char *
+qemuMigrationBeginPhase(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+const char *xmlin,
+const char *dname,
+char **cookieout,
+int *cookieoutlen,
+unsigned long flags)
 {
 char *rv = NULL;
 qemuMigrationCookiePtr mig = NULL;
-- 
1.9.1

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


  1   2   >