Re: [libvirt] [PATCH] storage: Disallow wiping an extended disk partition
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
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
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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
*** 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
... +/* 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
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
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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
--- 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
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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'
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*
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
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
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}
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
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