Re: [libvirt] [PATCH] apparmor: add missing qemu binaries

[Guido Günther]

Hi,
On Tue, Nov 24, 2015 at 03:20:00PM +, Daniel P. Berrange wrote:
> On Tue, Nov 24, 2015 at 04:16:11PM +0100, Guido Günther wrote:
> > This adds the qemu binaries as of 1.2.4 in Debian. It also removes a
> > duplicate sparc64 entry.
> > ---
> >  examples/apparmor/libvirt-qemu | 21 -
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> ACK

Pushed. Thanks!
 -- Guido

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

[libvirt] ANNOUNCE: virt-manager 1.3.0 released

[Cole Robinson]

I'm happy to announce the release of virt-manager 1.3.0!

virt-manager is a desktop application for managing KVM, Xen, and LXC
virtualization via libvirt.

The release can be downloaded from:

http://virt-manager.org/download/

This release includes:

- Git hosting moved to http://github.com/virt-manager/virt-manager
- Switch translation infrastructure from transifex to fedora.zanata.org
- Add dogtail UI tests and infrastructure
- Improved support for s390x kvm (Kevin Zhao)
- virt-install and virt-manager now remove created disk images if VM
  install startup fails
- Replace urlgrabber usage with requests and urllib2
- virt-install: add --network virtualport support for openvswitch
  (Daniel P. Berrange)
- virt-install: support multiple --security labels
- virt-install: support --features kvm_hidden=on|off (Pavel Hrdina)
- virt-install: add --features pmu=on|off
- virt-install: add --features pvspinlock=on|off (Abhijeet Kasurde)
- virt-install: add --events on_lockfailure=on|off (Abhijeet Kasurde)
- virt-install: add --network link_state=up|down
- virt-install: add --vcpu placement=static|auto

Thanks to everyone who has contributed to this release through testing,
bug reporting, submitting patches, and otherwise sending in feedback!

Thanks,
Cole

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

Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for perf event

[Ren, Qiaowei]

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Tuesday, November 24, 2015 5:43 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for 
> perf
> event
> 
> On Tue, Nov 17, 2015 at 16:00:44 +0800, Qiaowei Ren wrote:
> > Add remote support for perf event.
> >
> > Signed-off-by: Qiaowei Ren 
> > ---
> >  daemon/remote.c  | 60
> 
> >  src/remote/remote_driver.c   | 49
> 
> >  src/remote/remote_protocol.x | 32 ++-
> > src/remote_protocol-structs  | 20 +++
> >  4 files changed, 160 insertions(+), 1 deletion(-)
> 
> This will need to be changed to match the new style of the API. But, since you
> are adding a new API, would you mind creating a patch for libvirt-python to 
> add
> this new API to python bindings?
> 

Sure. The patch for libvirt-python should be also sent this maillist with this 
series, right?

Thanks,
Qiaowei 


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

[libvirt] [PATCH v2 2/5] storage: Add comments for backend APIs

[John Ferlan]

Just so it's clearer what to expect upon input and what types of return
values could be generated.  These were loosely copied from existing
virStorageBackendUpdateVolTargetInfoFD.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 38ef9fc..c870380 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1389,6 +1389,14 @@ static struct diskType const disk_types[] = {
 };
 
 
+/*
+ * virStorageBackendDetectBlockVolFormatFD
+ * @target: target definition ptr of volume to update
+ * @fd: fd of storage volume to update,
+ * @readflags: unused
+ *
+ * Returns 0 for success, -1 on a legitimate error condition
+ */
 static int
 virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
 int fd,
@@ -1578,6 +1586,17 @@ virStorageBackendVolOpen(const char *path, struct stat 
*sb,
 return fd;
 }
 
+/*
+ * virStorageBackendUpdateVolTargetInfo
+ * @target: target definition ptr of volume to update
+ * @withBlockVolFormat: true if caller determined a block file
+ * @openflags: various VolOpenCheckMode flags to handle errors on open
+ * @readflags: unused
+ *
+ * Returns 0 for success, -1 on a legitimate error condition, and -2
+ * if the openflags used VIR_STORAGE_VOL_OPEN_NOERROR and some sort of
+ * open error occurred. It is up to the caller to handle.
+ */
 int
 virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
  bool withBlockVolFormat,
@@ -1637,6 +1656,17 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr 
target,
 return ret;
 }
 
+/*
+ * virStorageBackendUpdateVolInfo
+ * @vol: Pointer to a volume storage definition
+ * @withBlockVolFormat: true if the caller determined a block file
+ * @openflags: various VolOpenCheckMode flags to handle errors on open
+ * @readflags: unused
+ *
+ * Returns 0 for success, -1 on a legitimate error condition, and -2
+ * if the openflags used VIR_STORAGE_VOL_OPEN_NOERROR and some sort of
+ * open error occurred. It is up to the caller to handle.
+ */
 int
 virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
bool withBlockVolFormat,
-- 
2.5.0

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

[libvirt] [PATCH v2 3/5] storage: Handle readflags errors

[John Ferlan]

Similar to the openflags VIR_STORAGE_VOL_OPEN_NOERROR processing, if some
read processing operation fails, check the readflags for the corresponding
error flag being set. If so, rather then causing an error - use VIR_WARN
to flag the error, but return -2 which some callers can use to perform
specific actions.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend.c | 107 +++---
 src/storage/storage_backend.h |  11 +
 2 files changed, 90 insertions(+), 28 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index c870380..da830d6 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1393,14 +1393,17 @@ static struct diskType const disk_types[] = {
  * virStorageBackendDetectBlockVolFormatFD
  * @target: target definition ptr of volume to update
  * @fd: fd of storage volume to update,
- * @readflags: unused
+ * @readflags: various VolReadErrorMode flags to handle errors after open
+ * is successful, but some other access/read is not.
  *
- * Returns 0 for success, -1 on a legitimate error condition
+ * Returns 0 for success, -1 on a legitimate error condition, -2 if
+ * some sort of error is desired to be ignored (along with appropriate
+ * VIR_WARN of the issue).
  */
 static int
 virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
 int fd,
-unsigned int readflags 
ATTRIBUTE_UNUSED)
+unsigned int readflags)
 {
 size_t i;
 off_t start;
@@ -1412,17 +1415,29 @@ 
virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
 
 start = lseek(fd, 0, SEEK_SET);
 if (start < 0) {
-virReportSystemError(errno,
- _("cannot seek to beginning of file '%s'"),
- target->path);
-return -1;
+if (readflags & VIR_STORAGE_VOL_SEEK_ERROR) {
+VIR_WARN("ignoring failed beginning of file lseek for '%s'",
+ target->path);
+return -2;
+} else {
+virReportSystemError(errno,
+ _("cannot seek to beginning of file '%s'"),
+ target->path);
+return -1;
+}
 }
 bytes = saferead(fd, buffer, sizeof(buffer));
 if (bytes < 0) {
-virReportSystemError(errno,
- _("cannot read beginning of file '%s'"),
- target->path);
-return -1;
+if (readflags & VIR_STORAGE_VOL_READ_ERROR) {
+VIR_WARN("ignoring failed saferead of file '%s'",
+ target->path);
+return -2;
+} else {
+virReportSystemError(errno,
+ _("cannot read beginning of file '%s'"),
+ target->path);
+return -1;
+}
 }
 
 for (i = 0; disk_types[i].part_table_type != -1; i++) {
@@ -1591,11 +1606,13 @@ virStorageBackendVolOpen(const char *path, struct stat 
*sb,
  * @target: target definition ptr of volume to update
  * @withBlockVolFormat: true if caller determined a block file
  * @openflags: various VolOpenCheckMode flags to handle errors on open
- * @readflags: unused
+ * @readflags: various VolReadErrorMode flags to handle errors after open
+ * is successful, but some other access/read is not.
  *
  * Returns 0 for success, -1 on a legitimate error condition, and -2
  * if the openflags used VIR_STORAGE_VOL_OPEN_NOERROR and some sort of
- * open error occurred. It is up to the caller to handle.
+ * open error occurred. It is up to the caller to handle. A -2 may also
+ * be returned if the caller passed a specific readflagsflag.
  */
 int
 virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
@@ -1625,17 +1642,36 @@ 
virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
 }
 
 if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
-virReportSystemError(errno, _("cannot seek to start of '%s'"), 
target->path);
+if (readflags & VIR_STORAGE_VOL_SEEK_ERROR) {
+VIR_WARN("ignoring failed beginning of file lseek for '%s'",
+ target->path);
+ret = -2;
+} else {
+virReportSystemError(errno,
+ _("cannot seek to start of '%s'"),
+ target->path);
+ret = -1;
+}
 goto cleanup;
 }
 
 if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) {
-virReportSystemError(errno, _("cannot read header '%s'"), 
target->path);
+if (readflags & VIR_STORAGE_VOL_READ_ERROR) {
+VIR_WARN("ignoring failed header read for '%s'",
+ target->path);
+

[libvirt] [PATCH v2 5/5] storage: Ignore block devices that fail format detection

[John Ferlan]

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

Prior to commit id '98322052' failure to saferead the block device would
cause an error to be logged and the device to be skipped while attempting
to discover/create a stable target path for a new LUN (NPIV).

This was because virStorageBackendSCSIFindLUs ignored errors from
processLU and virStorageBackendSCSINewLun.

Ignoring the failure allowed a multipath device with an "active" and
"ghost" to be present on the host with the "ghost" block device being
ignored. This patch will return a -2 to the caller indicating the desire
to ignore the block device since it cannot be used directly rather than
fail the pool startup.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_scsi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c 
b/src/storage/storage_backend_scsi.c
index cc2c5d7..27c90f6 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -224,8 +224,10 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
 goto cleanup;
 }
 
-if (virStorageBackendUpdateVolInfo(vol, true,
-   VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
+/* Allow a volume read failure to ignore or skip this block file */
+if ((retval = virStorageBackendUpdateVolInfo(vol, true,
+ VIR_STORAGE_VOL_OPEN_DEFAULT,
+ VIR_STORAGE_VOL_READ_ERROR)) 
< 0)
 goto cleanup;
 
 if (!(vol->key = virStorageBackendSCSISerial(vol->target.path)))
-- 
2.5.0

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

[libvirt] [PATCH v2 4/5] storage: Add debug message

[John Ferlan]

I found this useful while processing a volume that wouldn't end up
showing up in the resulting list of block volumes. In this case, the
partition type wasn't found in the disk_types table.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index da830d6..d7ba916 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1450,6 +1450,10 @@ 
virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
 }
 }
 
+if (target->format == VIR_STORAGE_POOL_DISK_UNKNOWN)
+VIR_DEBUG("cannot determine the target format for '%s'",
+  target->path);
+
 return 0;
 }
 
-- 
2.5.0

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

[libvirt] [PATCH v2 1/5] storage: Add readflags for backend error processing

[John Ferlan]

Similar to the openflags which allow VIR_STORAGE_VOL_OPEN_NOERROR to be
passed to avoid open errors, add a 'readflags' variable so that in the
future various read failures could also be ignored.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend.c | 24 
 src/storage/storage_backend.h |  9 ++---
 src/storage/storage_backend_disk.c|  5 +++--
 src/storage/storage_backend_fs.c  |  8 
 src/storage/storage_backend_gluster.c |  2 +-
 src/storage/storage_backend_logical.c |  2 +-
 src/storage/storage_backend_mpath.c   |  2 +-
 src/storage/storage_backend_scsi.c|  2 +-
 8 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 194736b..38ef9fc 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1391,7 +1391,8 @@ static struct diskType const disk_types[] = {
 
 static int
 virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
-int fd)
+int fd,
+unsigned int readflags 
ATTRIBUTE_UNUSED)
 {
 size_t i;
 off_t start;
@@ -1580,7 +1581,8 @@ virStorageBackendVolOpen(const char *path, struct stat 
*sb,
 int
 virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
  bool withBlockVolFormat,
- unsigned int openflags)
+ unsigned int openflags,
+ unsigned int readflags)
 {
 int ret, fd = -1;
 struct stat sb;
@@ -1592,7 +1594,8 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr 
target,
 goto cleanup;
 fd = ret;
 
-if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0)
+if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb,
+  readflags)) < 0)
 goto cleanup;
 
 if (target->type == VIR_STORAGE_VOL_FILE &&
@@ -1622,7 +1625,8 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr 
target,
 }
 
 if (withBlockVolFormat) {
-if ((ret = virStorageBackendDetectBlockVolFormatFD(target, fd)) < 0)
+if ((ret = virStorageBackendDetectBlockVolFormatFD(target, fd,
+   readflags)) < 0)
 goto cleanup;
 }
 
@@ -1636,20 +1640,22 @@ 
virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target,
 int
 virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
bool withBlockVolFormat,
-   unsigned int openflags)
+   unsigned int openflags,
+   unsigned int readflags)
 {
 int ret;
 
 if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target,
 withBlockVolFormat,
-openflags)) < 0)
+openflags, readflags)) < 0)
 return ret;
 
 if (vol->target.backingStore &&
 (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
 withBlockVolFormat,
 
VIR_STORAGE_VOL_OPEN_DEFAULT |
-
VIR_STORAGE_VOL_OPEN_NOERROR) < 0))
+
VIR_STORAGE_VOL_OPEN_NOERROR,
+readflags) < 0))
 return ret;
 
 return 0;
@@ -1660,13 +1666,15 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
  * @target: target definition ptr of volume to update
  * @fd: fd of storage volume to update, via virStorageBackendOpenVol*, or -1
  * @sb: details about file (must match @fd, if that is provided)
+ * @readflags: unused
  *
  * Returns 0 for success, -1 on a legitimate error condition.
  */
 int
 virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
int fd,
-   struct stat *sb)
+   struct stat *sb,
+   unsigned int readflags ATTRIBUTE_UNUSED)
 {
 #if WITH_SELINUX
 security_context_t filecon = NULL;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 96b5f39..aa9008e 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -192,13 +192,16 @@ int virStorageBackendVolOpen(const char *path, struct 
stat *sb,
 
 int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
bool withBlockVolFormat,
-   unsigned int openflags);
+   

[libvirt] [PATCH v2 0/5] Add read error ignore to storage backend vol info APIs

[John Ferlan]

v1 took a simplistic approach:

http://www.redhat.com/archives/libvir-list/2015-October/msg00919.html

but not specific enough according to the review. So adding some extra
complexity of checking for specific errors and flag bits to "ignore" the
error similar to the catch-all openflags VIR_STORAGE_VOL_OPEN_NOERROR for
all the virStorageBackendUpdateVol{Info|TargetInfo|TargetInfoFD} API's
which are used to get the volume capacity, allocation, etc. data.

Along the way an inavertent bug was found and fixed in patch 3 in
virStorageBackendUpdateVolTargetInfo where an error could have been
reported, but not propagated back to the user since 'ret' would have
been 0 after the virStorageBackendUpdateVolTargetInfoFD call.


John Ferlan (5):
  storage: Add readflags for backend error processing
  storage: Add comments for backend APIs
  storage: Handle readflags errors
  storage: Add debug message
  storage: Ignore block devices that fail format detection

 src/storage/storage_backend.c | 151 +++---
 src/storage/storage_backend.h |  20 -
 src/storage/storage_backend_disk.c|   5 +-
 src/storage/storage_backend_fs.c  |   8 +-
 src/storage/storage_backend_gluster.c |   2 +-
 src/storage/storage_backend_logical.c |   2 +-
 src/storage/storage_backend_mpath.c   |   2 +-
 src/storage/storage_backend_scsi.c|   6 +-
 8 files changed, 153 insertions(+), 43 deletions(-)

-- 
2.5.0

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

Re: [libvirt] how to know if PCI device has SR-IOV PF capability

[Moshe Levi]

Hi Laine,

Did you have a bug for this issue? 
I need it to document it in for my workaround in OpenStack. 
Thanks,
Moshe Levi. 

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Monday, November 23, 2015 6:29 PM
> To: Laine Stump 
> Cc: libvir-list@redhat.com; Moshe Levi 
> Subject: Re: [libvirt] how to know if PCI device has SR-IOV PF capability
> 
> On Mon, Nov 23, 2015 at 11:26:11AM -0500, Laine Stump wrote:
> > On 11/23/2015 10:40 AM, Daniel P. Berrange wrote:
> > >On Mon, Nov 23, 2015 at 10:34:43AM -0500, Laine Stump wrote:
> > >
> > >>If we're going to switch to emiting virt_functions whenever a device
> > >>has the potential to provide VFs, we may as well make it worthwhile
> > >>and also emit the maximum possible VFs for the device, maybe simply:
> > >>
> > >> 
> > >>
> > >>(the current count is implicit in the number of entries in the list
> > >>that follows. I don't have an opinion on whether it is better to
> > >>also include explicitly with, e.g. "count='7'", or just leave it 
> > >>implicit).
> > >Is there any way for us to actually discover the max count ? If so,
> > >then it seems nice to include it.
> >
> > Yes. I don't know if it existed back when that code was originally
> > added, but at least RHEL6.7 (the oldest OS I have running on a machine
> > with an SRIOV-capable card) and later have two files in the device's
> > sysfs, sriov_numvfs and sriov_totalvfs. The former is the number that
> > are currently active, and the latter is the maximum possible for this PF.
> >
> > (On a related topic - you can change the number of currently active
> > VFs by writing "0" to sriov_numvfs then writing the desired number to
> > it; this does temporarily delete any VFs that are already active
> > though, so it can only be done if none are in use. I've planned to
> > hook libvirt networks up to this so that VFs can be enabled completely
> > within libvirt (since the driver commandline method isn't consistent
> > between different vendors, and I believe is now considered to be
> > deprecated). Since Openstack doesn't use libvirt networks but may want
> > similar functionality, I'm wondering where would be a good place to do
> > that. We could provide something via the node-device API, but that
> > couldn't be automatically done by libvirtd at startup; alternately
> > Openstack could create a network but not use it, but that just seems
> > conceptually confusing even though it would work.)
> 
> There is scope to extend node device APIs to allow definition of persistent
> config for virtual devices. We have similar scenario wrt NPIV devices which 
> are
> dynamically allocatable
> 
> 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 java] Add support for Libvirt Qemu Library

[Wido den Hollander]

On 11/17/2015 08:43 AM, Claudio Bley wrote:
> At Mon, 16 Nov 2015 21:26:57 +0100,
> Wido den Hollander wrote:
>>
>>
>>
>>> Since there are no flags currently, simply drop the flags
>>> parameter. We add flags (as enums) later when the function starts to
>>> support one.
>>>
>>
>> Good point, I'll come up with a revised patch.
>>
>>> Furthermore, I think I'm in favor of adding a handful of methods
>>> instead of (ab)using some special values of a parameter.
>>>
>>> /* blocking call */
>>> String qemuAgentCommand(String cmd)
>>>
>>> /* default timeout call */
>>> String qemuAgentCommandDefaultTimeout(String cmd)
>>>
>>> /* timeout in seconds. no special 0 constant needed. */
>>> String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) {
>>>   if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds 
>>> value must be >= 0");
>>>
>>>   ...
>>> }
>>>
>>> Do you have a link to some docs where the available commands are
>>> described? We should add this to the javadoc.
>>>
>>
>> Yes, here they are: http://wiki.qemu.org/Features/QAPI/GuestAgent
>>
>>> What does a qemu command look like? It seems it's some kind of JSON
>>> object. In that case, I think we should not make every user of the
>>> library start to construct their own JSON strings.
>>>
>>> May be a simple utility class, e.g.
>>>
>>> public class QemuCommand {
>>>   public static QemuCommand create(String cmd, String args...) {
>>> ...
>>>   }
>>> }
>>>
>>> should be added and the qemuAgentCommand changed to take such a
>>> parameter instead?
>>>
>>
>> Hmm, I don't know if we want to do that. I think we should always keep
>> the option open for the user. Libvirt doesn't provide it. We are only
>> mapping libvirt functions.
> 
> Yes, but that doesn't mean we should not improve it, IMO. Thankfully,
> in Java, we're not bound by the limitations of the C language. Ease of
> use of the API is one of my foremost goals for libvirt-java. A plain
> mapping of the C API functions would be far easier, but also downright
> ugly (with the Java goggles on).
> 
> Having a (bit) of Haskell background, and being a Scala programmer by
> day, a plain String in place of a proper type is just ugly as well, as
> it does not carry any information as to what it's type really is and
> what it's format is.
> 
> I see that the JSON structure of an agent command is pretty simple,
> but we would have to support strings and numbers. I'd still very much
> prefer a QemuCommand helper class.
> 

True, I understand. Looking at this I would want to use the
JSONReader/JsonWriter from Java, but that's not available by default.

We probably don't want any external libs for libvirt-java. 'arguments'
for Qemu GA is rather simple, it's an array with Strings or Ints.

I rather not write my own JsonWriter, so any ideas here?

> If it turns out that it is not flexible enough, we can always add a
> loophole later, e.g.:
> 

Indeed. We could also implement all available commands as a Enum:
http://git.qemu.org/?p=qemu.git;a=blob;f=qga/qapi-schema.json;h=78362e071d18de1f04f3b5730d8a32dcb46ee74c;hb=HEAD

It's just the arguments I'm stuck with. I have a few commits, do you
care to take a look at them:
https://github.com/wido/libvirt-java/commits/qemu-guest-command

Especially this one:
https://github.com/wido/libvirt-java/commit/1ff76ed0a8f8d58a102cd4e2bc948df99a472e20

Wido

> class QemuCommand {
>   public static QemuCommand execute(String cmd, /* TBD */ args...) {
>   }
> 
>   public static QemuCommand raw(String jsonObject) {
>   }
> }
> 
 Yes, that would indeed fail. What do you suggest? Load when required? My
 JNA foo is not that good to get this implemented.
>>>
>>> We can probably use some sort of singleton pattern
>>> (e.g. initialization-on-demand holder).
>>>
>>
>> I might need some help with that! I'll come up with a revised patched
>> based on your comments.
> 
> OK, I'm sure we can work that out. Basically, we probably need to
> access the qemu library through a static method, so we avoid loading
> the library when it is not necessary. Let me think about that a bit
> more.
> 
> --
> Claudio-- 
> 

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

Re: [libvirt] [libvirt-glib v4] gobject: Port to GTask API

[Zeeshan Ali (Khattak)]

On Tue, Nov 24, 2015 at 4:23 PM, Zeeshan Ali (Khattak)
 wrote:
> Drop usage of deprecated GSimpleAsyncResult API.
> ---

This version ensures that async result is returned from the main
thread in gvir_stream_close_async().

>  libvirt-gobject/libvirt-gobject-domain.c| 290 
> +---
>  libvirt-gobject/libvirt-gobject-input-stream.c  |  77 +++
>  libvirt-gobject/libvirt-gobject-output-stream.c |  75 +++---
>  libvirt-gobject/libvirt-gobject-storage-pool.c  | 281 ++-
>  libvirt-gobject/libvirt-gobject-stream.c|  53 +++--
>  5 files changed, 322 insertions(+), 454 deletions(-)
>
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
> b/libvirt-gobject/libvirt-gobject-domain.c
> index 5509ce3..c768cd3 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -370,28 +370,20 @@ gboolean gvir_domain_start(GVirDomain *dom,
>  return TRUE;
>  }
>
> -typedef struct {
> -guint flags;
> -} DomainStartData;
> -
> -static void domain_start_data_free(DomainStartData *data)
> -{
> -g_slice_free(DomainStartData, data);
> -}
> -
>  static void
> -gvir_domain_start_helper(GSimpleAsyncResult *res,
> - GObject *object,
> +gvir_domain_start_helper(GTask *task,
> + gpointer source_object,
> + gpointer task_data,
>   GCancellable *cancellable G_GNUC_UNUSED)
>  {
> -GVirDomain *dom = GVIR_DOMAIN(object);
> -DomainStartData *data;
> +GVirDomain *dom = GVIR_DOMAIN(source_object);
> +guint flags = GPOINTER_TO_UINT(task_data);
>  GError *err = NULL;
>
> -data = g_simple_async_result_get_op_res_gpointer(res);
> -
> -if (!gvir_domain_start(dom, data->flags, &err))
> -g_simple_async_result_take_error(res, err);
> +if (!gvir_domain_start(dom, flags, &err))
> +g_task_return_error(task, err);
> +else
> +g_task_return_boolean(task, TRUE);
>  }
>
>  /**
> @@ -410,25 +402,18 @@ void gvir_domain_start_async(GVirDomain *dom,
>   GAsyncReadyCallback callback,
>   gpointer user_data)
>  {
> -GSimpleAsyncResult *res;
> -DomainStartData *data;
> +GTask *task;
>
>  g_return_if_fail(GVIR_IS_DOMAIN(dom));
>  g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
>
> -data = g_slice_new0(DomainStartData);
> -data->flags = flags;
> -
> -res = g_simple_async_result_new(G_OBJECT(dom),
> -callback,
> -user_data,
> -gvir_domain_start_async);
> -g_simple_async_result_set_op_res_gpointer (res, data, 
> (GDestroyNotify)domain_start_data_free);
> -g_simple_async_result_run_in_thread(res,
> -gvir_domain_start_helper,
> -G_PRIORITY_DEFAULT,
> -cancellable);
> -g_object_unref(res);
> +task = g_task_new(G_OBJECT(dom),
> +  cancellable,
> +  callback,
> +  user_data);
> +g_task_set_task_data(task, GUINT_TO_POINTER(flags), NULL);
> +g_task_run_in_thread(task, gvir_domain_start_helper);
> +g_object_unref(task);
>  }
>
>  gboolean gvir_domain_start_finish(GVirDomain *dom,
> @@ -436,13 +421,10 @@ gboolean gvir_domain_start_finish(GVirDomain *dom,
>GError **err)
>  {
>  g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
> -g_return_val_if_fail(g_simple_async_result_is_valid(result, 
> G_OBJECT(dom), gvir_domain_start_async), FALSE);
> +g_return_val_if_fail(g_task_is_valid(result, dom), FALSE);
>  g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
>
> -if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), 
> err))
> -return FALSE;
> -
> -return TRUE;
> +return g_task_propagate_boolean(G_TASK(result), err);
>  }
>
>  /**
> @@ -472,15 +454,18 @@ gboolean gvir_domain_resume(GVirDomain *dom,
>  }
>
>  static void
> -gvir_domain_resume_helper(GSimpleAsyncResult *res,
> -  GObject *object,
> +gvir_domain_resume_helper(GTask *task,
> +  gpointer source_object,
> +  gpointer task_data G_GNUC_UNUSED,
>GCancellable *cancellable G_GNUC_UNUSED)
>  {
> -GVirDomain *dom = GVIR_DOMAIN(object);
> +GVirDomain *dom = GVIR_DOMAIN(source_object);
>  GError *err = NULL;
>
>  if (!gvir_domain_resume(dom, &err))
> -g_simple_async_result_take_error(res, err);
> +g_task_return_error(task, err);
> +else
> +g_task_return_boolean(task, TRUE);
>  }
>
>  /**
> @@ -497,20 +482,17 @@ void gvir_domain_resume_async(GVirDomain *dom,
>GAsyncReadyCall

[libvirt] [libvirt-glib v4] gobject: Port to GTask API

[Zeeshan Ali (Khattak)]

Drop usage of deprecated GSimpleAsyncResult API.
---
 libvirt-gobject/libvirt-gobject-domain.c| 290 +---
 libvirt-gobject/libvirt-gobject-input-stream.c  |  77 +++
 libvirt-gobject/libvirt-gobject-output-stream.c |  75 +++---
 libvirt-gobject/libvirt-gobject-storage-pool.c  | 281 ++-
 libvirt-gobject/libvirt-gobject-stream.c|  53 +++--
 5 files changed, 322 insertions(+), 454 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 5509ce3..c768cd3 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -370,28 +370,20 @@ gboolean gvir_domain_start(GVirDomain *dom,
 return TRUE;
 }
 
-typedef struct {
-guint flags;
-} DomainStartData;
-
-static void domain_start_data_free(DomainStartData *data)
-{
-g_slice_free(DomainStartData, data);
-}
-
 static void
-gvir_domain_start_helper(GSimpleAsyncResult *res,
- GObject *object,
+gvir_domain_start_helper(GTask *task,
+ gpointer source_object,
+ gpointer task_data,
  GCancellable *cancellable G_GNUC_UNUSED)
 {
-GVirDomain *dom = GVIR_DOMAIN(object);
-DomainStartData *data;
+GVirDomain *dom = GVIR_DOMAIN(source_object);
+guint flags = GPOINTER_TO_UINT(task_data);
 GError *err = NULL;
 
-data = g_simple_async_result_get_op_res_gpointer(res);
-
-if (!gvir_domain_start(dom, data->flags, &err))
-g_simple_async_result_take_error(res, err);
+if (!gvir_domain_start(dom, flags, &err))
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
 }
 
 /**
@@ -410,25 +402,18 @@ void gvir_domain_start_async(GVirDomain *dom,
  GAsyncReadyCallback callback,
  gpointer user_data)
 {
-GSimpleAsyncResult *res;
-DomainStartData *data;
+GTask *task;
 
 g_return_if_fail(GVIR_IS_DOMAIN(dom));
 g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
 
-data = g_slice_new0(DomainStartData);
-data->flags = flags;
-
-res = g_simple_async_result_new(G_OBJECT(dom),
-callback,
-user_data,
-gvir_domain_start_async);
-g_simple_async_result_set_op_res_gpointer (res, data, 
(GDestroyNotify)domain_start_data_free);
-g_simple_async_result_run_in_thread(res,
-gvir_domain_start_helper,
-G_PRIORITY_DEFAULT,
-cancellable);
-g_object_unref(res);
+task = g_task_new(G_OBJECT(dom),
+  cancellable,
+  callback,
+  user_data);
+g_task_set_task_data(task, GUINT_TO_POINTER(flags), NULL);
+g_task_run_in_thread(task, gvir_domain_start_helper);
+g_object_unref(task);
 }
 
 gboolean gvir_domain_start_finish(GVirDomain *dom,
@@ -436,13 +421,10 @@ gboolean gvir_domain_start_finish(GVirDomain *dom,
   GError **err)
 {
 g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
-g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(dom), 
gvir_domain_start_async), FALSE);
+g_return_val_if_fail(g_task_is_valid(result, dom), FALSE);
 g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
 
-if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), 
err))
-return FALSE;
-
-return TRUE;
+return g_task_propagate_boolean(G_TASK(result), err);
 }
 
 /**
@@ -472,15 +454,18 @@ gboolean gvir_domain_resume(GVirDomain *dom,
 }
 
 static void
-gvir_domain_resume_helper(GSimpleAsyncResult *res,
-  GObject *object,
+gvir_domain_resume_helper(GTask *task,
+  gpointer source_object,
+  gpointer task_data G_GNUC_UNUSED,
   GCancellable *cancellable G_GNUC_UNUSED)
 {
-GVirDomain *dom = GVIR_DOMAIN(object);
+GVirDomain *dom = GVIR_DOMAIN(source_object);
 GError *err = NULL;
 
 if (!gvir_domain_resume(dom, &err))
-g_simple_async_result_take_error(res, err);
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
 }
 
 /**
@@ -497,20 +482,17 @@ void gvir_domain_resume_async(GVirDomain *dom,
   GAsyncReadyCallback callback,
   gpointer user_data)
 {
-GSimpleAsyncResult *res;
+GTask *task;
 
 g_return_if_fail(GVIR_IS_DOMAIN(dom));
 g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
 
-res = g_simple_async_result_new(G_OBJECT(dom),
-callback,
-user_data,
-   

Re: [libvirt] [PATCH] apparmor: add missing qemu binaries

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 04:16:11PM +0100, Guido Günther wrote:
> This adds the qemu binaries as of 1.2.4 in Debian. It also removes a
> duplicate sparc64 entry.
> ---
>  examples/apparmor/libvirt-qemu | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)

ACK

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

[libvirt] [PATCH] apparmor: add missing qemu binaries

[Guido Günther]

This adds the qemu binaries as of 1.2.4 in Debian. It also removes a
duplicate sparc64 entry.
---
 examples/apparmor/libvirt-qemu | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index c80ece7..efb4873 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -75,9 +75,12 @@
   /usr/bin/kvm rmix,
   /usr/bin/qemu rmix,
   /usr/bin/qemu-kvm rmix,
+  /usr/bin/qemu-system-aarch64 rmix,
+  /usr/bin/qemu-system-alpha rmix,
   /usr/bin/qemu-system-arm rmix,
   /usr/bin/qemu-system-cris rmix,
   /usr/bin/qemu-system-i386 rmix,
+  /usr/bin/qemu-system-lm32 rmix,
   /usr/bin/qemu-system-m68k rmix,
   /usr/bin/qemu-system-microblaze rmix,
   /usr/bin/qemu-system-microblazeel rmix,
@@ -85,14 +88,22 @@
   /usr/bin/qemu-system-mips64 rmix,
   /usr/bin/qemu-system-mips64el rmix,
   /usr/bin/qemu-system-mipsel rmix,
+  /usr/bin/qemu-system-moxie rmix,
+  /usr/bin/qemu-system-or32 rmix,
   /usr/bin/qemu-system-ppc rmix,
   /usr/bin/qemu-system-ppc64 rmix,
   /usr/bin/qemu-system-ppcemb rmix,
+  /usr/bin/qemu-system-s390x rmix,
   /usr/bin/qemu-system-sh4 rmix,
   /usr/bin/qemu-system-sh4eb rmix,
   /usr/bin/qemu-system-sparc rmix,
   /usr/bin/qemu-system-sparc64 rmix,
+  /usr/bin/qemu-system-tricore rmix,
+  /usr/bin/qemu-system-unicore32 rmix,
   /usr/bin/qemu-system-x86_64 rmix,
+  /usr/bin/qemu-system-xtensa rmix,
+  /usr/bin/qemu-system-xtensaeb rmix,
+  /usr/bin/qemu-aarch64 rmix,
   /usr/bin/qemu-alpha rmix,
   /usr/bin/qemu-arm rmix,
   /usr/bin/qemu-armeb rmix,
@@ -102,16 +113,24 @@
   /usr/bin/qemu-microblaze rmix,
   /usr/bin/qemu-microblazeel rmix,
   /usr/bin/qemu-mips rmix,
+  /usr/bin/qemu-mips64 rmix,
+  /usr/bin/qemu-mips64el rmix,
   /usr/bin/qemu-mipsel rmix,
+  /usr/bin/qemu-mipsn32 rmix,
+  /usr/bin/qemu-mipsn32el rmix,
+  /usr/bin/qemu-nbd rmix,
+  /usr/bin/qemu-or32 rmix,
   /usr/bin/qemu-ppc rmix,
   /usr/bin/qemu-ppc64 rmix,
   /usr/bin/qemu-ppc64abi32 rmix,
+  /usr/bin/qemu-ppc64le rmix,
+  /usr/bin/qemu-s390x rmix,
   /usr/bin/qemu-sh4 rmix,
   /usr/bin/qemu-sh4eb rmix,
   /usr/bin/qemu-sparc rmix,
-  /usr/bin/qemu-sparc64 rmix,
   /usr/bin/qemu-sparc32plus rmix,
   /usr/bin/qemu-sparc64 rmix,
+  /usr/bin/qemu-unicore32 rmix,
   /usr/bin/qemu-x86_64 rmix,
   /usr/{lib,lib64}/qemu/block-curl.so mr,
   /usr/{lib,lib64}/qemu/block-rbd.so mr,
-- 
2.6.2

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

Re: [libvirt] [libvirt-glib v3] gobject: Port to GTask API

[Christophe Fergeau]

On Mon, Nov 23, 2015 at 10:50:48PM +, Zeeshan Ali (Khattak) wrote:
> On Mon, Nov 23, 2015 at 10:45 PM, Zeeshan Ali (Khattak)
>  wrote:
> > Drop usage of deprecated GSimpleAsyncResult API.
> > ---
> 
> Sorry, forgot to pass --annotate option to git-send-email.
> 
> v3 just drops some of the private context structures in favour of
> GPOINTER_TO_UINT/GUINT_TO_POINTER usage.

What about
https://www.redhat.com/archives/libvir-list/2015-November/msg00897.html
?

Christophe


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

Re: [libvirt] [PATCH] storage: Ignore block devices that fail format detection

[John Ferlan]

On 11/23/2015 09:13 AM, Ján Tomko wrote:
> On Fri, Oct 30, 2015 at 11:13:21AM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1276198
>>
>> Prior to commit id '98322052' failure to saferead the block device would
>> cause an error to be logged and the device to be skipped while attempting
>> to discover/create a stable target path for a new LUN (NPIV).
>>
>> This was because virStorageBackendSCSIFindLUs ignored errors from
>> processLU and virStorageBackendSCSINewLun.
>>
>> Ignoring the failure allowed a multipath device with an "active" and
>> "ghost" to be present on the host with the "ghost" block device being
>> ignored. This patch will return a -2 to the caller indicating the desire
>> to ignore the block device since it cannot be used directly rather than
>> fail the pool startup.
>>
>> Additionally, it was found during some debugging that it was possible
>> for the virStorageBackendDetectBlockVolFormatFD to not detect a format,
>> which while not a probably - we probably should at least add some sort
>> of warning message.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/storage/storage_backend.c  | 4 
>>  src/storage/storage_backend_scsi.c | 7 ++-
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>> index a375fe0..2de606f 100644
>> --- a/src/storage/storage_backend.c
>> +++ b/src/storage/storage_backend.c
>> @@ -1393,6 +1393,10 @@ 
>> virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
>>  }
>>  }
>>  
>> +if (target->format == VIR_STORAGE_POOL_DISK_UNKNOWN)
>> +VIR_WARN("cannot determine the target format for '%s'",
>> + target->path);
>> +
>>  return 0;
>>  }
>>  
> 
> This change is unrelated to the other one. Also, is this warning really
> needed? I think leaving the format as 'unknown' is visible enough.
> 

No not needed - I can remove it or change to VIR_DEBUG - it was
certainly useful when debugging.

>> diff --git a/src/storage/storage_backend_scsi.c 
>> b/src/storage/storage_backend_scsi.c
>> index a593a2b..d60473d 100644
>> --- a/src/storage/storage_backend_scsi.c
>> +++ b/src/storage/storage_backend_scsi.c
>> @@ -224,9 +224,14 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>>  goto cleanup;
>>  }
>>  
>> +/* Failing to process the format is not fatal - we'll just skip
>> + * this volume.
>> + */
>>  if (virStorageBackendUpdateVolInfo(vol, true,
>> -   VIR_STORAGE_VOL_OPEN_DEFAULT) < 0)
>> +   VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
>> +retval = -2;
>>  goto cleanup;
> 
> Adding the VIR_STORAGE_VOL_OPEN_NOERROR flag and propagating it down to
> virStorageBackendDetectBlockVolFormatFD would be nicer.

So it's not an OPEN failure rather this a READ failure. The volume could
be opened and passing OPEN_NOERROR along in the "more general" is
probably not what should be done as that opens up a new classification
of errors that could get by.

I'll work on adding a similar read noerror setting

John

> 
> That way it can return -2 on EIO if the flag is present without spamming
> the logs. The other functions called by virStorageBackendUpdateVolInfo
> can also return -1 on a fatal error that way (although there do not seem
> to be any functions that could fail with NOERROR at the moment).
> 
> Jan
> 

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

Re: [libvirt] [PATCH 5/6] RFC qemu: add spice opengl support

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 03:50:02PM +0100, Marc-André Lureau wrote:
> Hi Jan
> 
> On Mon, Nov 23, 2015 at 6:58 PM, Ján Tomko  wrote:
> > Overall, the XML looks good to me. Apart from the accel3d attribute,
> > which is ugly, but less ugly than inveting a separate one.
> 
> Thanks for your feedback.
> 
> While reworking the patches and adding xml2xml test, I realized the
> accel2d/3d is a simple bool, so
>   
>   
>   
> 
> Is turned into:
>
>   
> 
> 
> Would it be backward incompatible if we changed it to a tristate?

I think making it a tristate is fine and in fact very desirable


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] build issue due to '-Werror=cast-align' on ARM (armhf) [was: Re: [Xen-devel] [libvirt test] 63528: regressions - FAIL]

[Ian Campbell]

Hi Maxim,

On Fri, 2015-11-13 at 20:26 -0700, Jim Fehlig wrote:
> On 11/13/2015 10:00 AM, Dario Faggioli wrote:
> > Hello,
> > 
> > The Xen Project's automated test suite is failing at running its
> > libvirt tests for a few time, like this:
> > 
> > On Wed, 2015-11-04 at 09:04 +, osstest service owner wrote:
> > > flight 63528 libvirt real [real]
> > > http://logs.test-lab.xenproject.org/osstest/logs/63528/
> > > 
> > > Regressions :-(
> > > 
> > > Tests which did not succeed and are blocking,
> > > including tests which could not be run:
> > >  build-armhf-libvirt   5 libvirt-build fail REGR.
> > > vs. 63340
> > > 
> > More specifically (from a more recent run):
> > 
> >   CC util/libvirt_util_la-virnetdevmacvlan.lo
> > util/virnetdev.c: In function 'virNetDevParseDadStatus':
> > util/virnetdev.c:1319:188: error: cast increases required alignment of 
> > target type [-Werror=cast-align]
> > util/virnetdev.c:1332:41: error: cast increases required alignment of 
> > target type [-Werror=cast-align]
> > util/virnetdev.c:1334:92: error: cast increases required alignment of 
> > target type [-Werror=cast-align]
> > cc1: all warnings being treated as errors
> > make[3]: *** [util/libvirt_util_la-virnetdev.lo] Error 1
> > make[3]: *** Waiting for unfinished jobs
> > make[3]: Leaving directory 
> > `/home/osstest/build.64130.build-armhf-libvirt/libvirt/src'
> > make[2]: *** [all] Error 2
> > make[2]: Leaving directory 
> > `/home/osstest/build.64130.build-armhf-libvirt/libvirt/src'
> > make[1]: *** [all-recursive] Error 1
> > make[1]: Leaving directory 
> > `/home/osstest/build.64130.build-armhf-libvirt/libvirt'
> > make: *** [all] Error 2


According to http://logs.test-lab.xenproject.org/osstest/logs/65035/build-a
rmhf-libvirt/info.html this build failure is still occurring with the same
trace.

Is there a fix in the works?

Ian.

> 
> Yep, noticed this several days ago but haven't yet found time to
> investigate.
> 
> > The changeset being tested last, as of now, and still failing, is:
> > db92aee2b477355759351889e37a365379b43bf6
> 
> I did notice the bisector ran and flagged commit 0f7436ca
> 
> http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg03385.html
> 
> I've cc'd author of that commit.
> 
> Regards,
> Jim
> 
> > 
> > Full logs available at the following links:
> > http://logs.test-lab.xenproject.org/osstest/logs/64130/build-armhf-libv
> > irt/info.html
> > http://logs.test-lab.xenproject.org/osstest/logs/64130/build-armhf-libv
> > irt/5.ts-libvirt-build.log
> > 
> > Right now, I don't have an ARM cross build environment handy to try to
> > reproduce and attempt a fix, so I figured I at least was reporting it
> > here.
> > 
> > Regards,
> > Dario
> 

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

Re: [libvirt] [PATCH 5/6] RFC qemu: add spice opengl support

[Marc-André Lureau]

Hi Jan

On Mon, Nov 23, 2015 at 6:58 PM, Ján Tomko  wrote:
> Overall, the XML looks good to me. Apart from the accel3d attribute,
> which is ugly, but less ugly than inveting a separate one.

Thanks for your feedback.

While reworking the patches and adding xml2xml test, I realized the
accel2d/3d is a simple bool, so
  
  
  

Is turned into:
   
  


Would it be backward incompatible if we changed it to a tristate?

thanks

-- 
Marc-André Lureau

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

Re: [libvirt] [PATCH] vz: implementation of domainCreateXML callback

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 05:24:48PM +0300, Dmitry Guryanov wrote:
> On Fri, 2015-11-20 at 16:52 +0300, Mikhail Feoktistov wrote:
> > ---
> >  src/vz/vz_driver.c | 33 +
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> > index 39f58a4..31dfb6a 100644
> > --- a/src/vz/vz_driver.c
> > +++ b/src/vz/vz_driver.c
> > @@ -979,6 +979,38 @@ vzDomainUndefine(virDomainPtr domain)
> >  return vzDomainUndefineFlags(domain, 0);
> >  }
> >  
> > +static virDomainPtr
> > +vzDomainCreateXML(virConnectPtr conn,
> > +  const char *xml,
> > +  unsigned int flags)
> > +{
> > +virDomainPtr domain;
> > +int ret;
> > +
> > +virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
> > +
> > +domain = vzDomainDefineXMLFlags(conn, xml, 0);
> > +if (domain == NULL)
> > +return domain;
> > +
> > +ret = vzDomainCreate(domain);
> > +if (ret != 0) {
> > +vzDomainUndefine(domain);
> > +return NULL;
> > +}
> > +
> > +if (flags & VIR_DOMAIN_START_PAUSED) {
> > +ret = vzDomainSuspend(domain);
> > +if (ret != 0) {
> > +vzDomainDestroy(domain);
> > +vzDomainUndefine(domain);
> > +return NULL;
> > +}
> 
> I think we shouldn't allow this start paused operation. Because it means that
> domain shouldn't run any code before being paused, and with this 
> implementation
> we start a domain, it does some work and only that you suspend it. Also 
> suspend
> and paused states are different and we only support paused state for VMs.

Yeah, VIR_DOMAIN_START_PAUSED really needs to guarantee that the
guest OS does not execute any code at all, not even a tiny bit.

If a driver can't do that, apps can still fallback to manually pausing
the guest after starting it, if they are ok with those semantics, so
no need for drivers todo that fallback approach themselves.

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] vz: implementation of domainCreateXML callback

[Dmitry Guryanov]

On Fri, 2015-11-20 at 16:52 +0300, Mikhail Feoktistov wrote:
> ---
>  src/vz/vz_driver.c | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 39f58a4..31dfb6a 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -979,6 +979,38 @@ vzDomainUndefine(virDomainPtr domain)
>  return vzDomainUndefineFlags(domain, 0);
>  }
>  
> +static virDomainPtr
> +vzDomainCreateXML(virConnectPtr conn,
> +  const char *xml,
> +  unsigned int flags)
> +{
> +virDomainPtr domain;
> +int ret;
> +
> +virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
> +
> +domain = vzDomainDefineXMLFlags(conn, xml, 0);
> +if (domain == NULL)
> +return domain;
> +
> +ret = vzDomainCreate(domain);
> +if (ret != 0) {
> +vzDomainUndefine(domain);
> +return NULL;
> +}
> +
> +if (flags & VIR_DOMAIN_START_PAUSED) {
> +ret = vzDomainSuspend(domain);
> +if (ret != 0) {
> +vzDomainDestroy(domain);
> +vzDomainUndefine(domain);
> +return NULL;
> +}

I think we shouldn't allow this start paused operation. Because it means that
domain shouldn't run any code before being paused, and with this implementation
we start a domain, it does some work and only that you suspend it. Also suspend
and paused states are different and we only support paused state for VMs.

> +}
> +
> +return domain;
> +}
> +
>  static int
>  vzDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags)
>  {
> @@ -1493,6 +1525,7 @@ static virHypervisorDriver vzDriver = {
>  .domainShutdown = vzDomainShutdown, /* 0.10.0 */
>  .domainCreate = vzDomainCreate,/* 0.10.0 */
>  .domainCreateWithFlags = vzDomainCreateWithFlags, /* 1.2.10 */
> +.domainCreateXML = vzDomainCreateXML, /* 1.2.22 */
>  .domainReboot = vzDomainReboot, /* 1.2.22 */
>  .domainDefineXML = vzDomainDefineXML,  /* 0.10.0 */
>  .domainDefineXMLFlags = vzDomainDefineXMLFlags, /* 1.2.12 */
-- 
Dmitry Guryanov

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

Re: [libvirt] [PATCH] vz: allow to boot VM from cdrom

[Dmitry Guryanov]

On Fri, 2015-11-20 at 16:00 +0300, Mikhail Feoktistov wrote:

Could you add some detail to commit message, because this change is not trivial?

> ---
>  src/vz/vz_sdk.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 750133d..9bdf7aa9a 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -2023,8 +2023,9 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom,
> virDomainDefPtr def)
>  }
>  
>  if (!IS_CT(def)) {
> -if (def->os.nBootDevs != 1 ||
> -def->os.bootDevs[0] != VIR_DOMAIN_BOOT_DISK ||
> +if (def->os.nBootDevs == 0 ||
> +(def->os.bootDevs[0] != VIR_DOMAIN_BOOT_DISK &&
> +def->os.bootDevs[0] != VIR_DOMAIN_BOOT_CDROM) ||
>  def->os.init != NULL || def->os.initargv != NULL) {
>  
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -3179,11 +3180,12 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom,
>  
>  if (disk->src->type == VIR_STORAGE_TYPE_FILE) {
>  if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
> -virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_PLOOP) {
> +(virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_PLOOP &&
> + virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_NONE)) {
>  

Why do you need this change?

>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid format of "
> - "disk %s, vz driver
> supports only "
> - "images in ploop
> format."), disk->src->path);
> + "disk %s, it should
> be either not set or "
> + "ploop format."),
> disk->src->path);
>  goto cleanup;
>  }
>  
> @@ -3205,11 +3207,13 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom,
>  pret = PrlVmDev_SetEmulatedType(sdkdisk, emutype);
>  prlsdkCheckRetGoto(pret, cleanup);
>  
> -pret = PrlVmDev_SetSysName(sdkdisk, disk->src->path);
> -prlsdkCheckRetGoto(pret, cleanup);
> +if (disk->src->path != NULL) {
> +pret = PrlVmDev_SetSysName(sdkdisk, disk->src->path);
> +prlsdkCheckRetGoto(pret, cleanup);
>  
> -pret = PrlVmDev_SetFriendlyName(sdkdisk, disk->src->path);
> -prlsdkCheckRetGoto(pret, cleanup);
> +pret = PrlVmDev_SetFriendlyName(sdkdisk, disk->src->path);
> +prlsdkCheckRetGoto(pret, cleanup);
> +}

Could you, please, also add some comment about this code. I I thought we can
only add disks with existing images. But here you allow to add disk without
source path, how will it work?

>  
>  drive = &disk->info.addr.drive;
>  if (drive->controller > 0) {
-- 
Dmitry Guryanov

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

Re: [libvirt] [PATCH] vz: Allow to create container based on template

[Dmitry Guryanov]

On Thu, 2015-11-19 at 14:44 +0300, Mikhail Feoktistov wrote:
> We shouldn't delete disk from default config if we create container based on
> template,
> because we don't have the new disk from XML, only template name.
> And don't add template section from XML as new filesystem,
> we use PrlVmCfg_SetOsTemplate function to set template name.
> ---
>  src/vz/vz_sdk.c | 33 +
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 89c9e89..865cabe 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -2096,12 +2096,14 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom,
> virDomainDefPtr def)
>  return 0;
>  }
>  
> -static int prlsdkClearDevices(PRL_HANDLE sdkdom)
> +static int prlsdkClearDevices(PRL_HANDLE sdkdom, bool skipdisk)
>  {
>  PRL_RESULT pret;
>  PRL_UINT32 n, i;
>  PRL_HANDLE devList;
>  PRL_HANDLE dev;
> +PRL_DEVICE_TYPE devType;
> +PRL_VM_DEV_EMULATION_TYPE emul;
>  int ret = -1;
>  
>  pret = PrlVmCfg_SetVNCMode(sdkdom, PRD_DISABLED);
> @@ -2117,6 +2119,18 @@ static int prlsdkClearDevices(PRL_HANDLE sdkdom)
>  pret = PrlHndlList_GetItem(devList, i, &dev);
>  prlsdkCheckRetGoto(pret, cleanup);
>  
> +if (skipdisk) {
> +pret = PrlVmDev_GetType(dev, &devType);
> +prlsdkCheckRetGoto(pret, cleanup);
> +
> +pret = PrlVmDev_GetEmulatedType(dev, &emul);

Where do you use this emul?

> +prlsdkCheckRetGoto(pret, cleanup);
> +
> +if (devType == PDE_HARD_DISK) {
> +PrlHandle_Free(dev);
> +continue;
> +}
> +}
>  pret = PrlVmDev_Remove(dev);
>  PrlHandle_Free(dev);
>  }
> @@ -3465,6 +3479,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
>  char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
>  bool needBoot = true;
>  char *mask = NULL;
> +bool skipdisk = false;
>  
>  if (prlsdkCheckUnsupportedParams(sdkdom, def) < 0)
>  return -1;
> @@ -3514,7 +3529,11 @@ prlsdkDoApplyConfig(virConnectPtr conn,
>  }
>  prlsdkCheckRetGoto(pret, error);
>  
> -if (prlsdkClearDevices(sdkdom) < 0)
> +if (def->nfss == 1 &&
> +def->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE)
> +skipdisk = true;
> +
> +if (prlsdkClearDevices(sdkdom, skipdisk) < 0)
>  goto error;
>  

I think we should make this logic more robust. There is only one case,
when VIR_DOMAIN_FS_TYPE_TEMPLATE fs is allowed - when we create new container.
So I'd add a parameter to prlsdkDoApplyConfig, something like useCtTemplateFs,
which means that we should have only one fs of type template and no disks.

>  if (prlsdkRemoveBootDevices(sdkdom) < 0)
> @@ -3544,6 +3563,8 @@ prlsdkDoApplyConfig(virConnectPtr conn,
>  for (i = 0; i < def->nfss; i++) {
>  if (STREQ(def->fss[i]->dst, "/"))
>  needBoot = false;
> +if (def->fss[i]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE)
> +continue;

If fs with index different from 0 is VIR_DOMAIN_FS_TYPE_TEMPLATE - it's an
error, also if we are not creating new ct - it's also error.

>  if (prlsdkAddFS(sdkdom, def->fss[i]) < 0)
>  goto error;
>  }
> @@ -3655,6 +3676,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
>  int ret = -1;
>  int useTemplate = 0;
>  size_t i;
> +PRL_UINT32 flags = 0;
>  
>  if (def->nfss > 1) {
>  /* Check all filesystems */
> @@ -3696,8 +3718,11 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
>  if (ret)
>  goto cleanup;
>  
> -job = PrlVm_RegEx(sdkdom, "",
> -  PACF_NON_INTERACTIVE_MODE | PRNVM_PRESERVE_DISK);
> +flags = PACF_NON_INTERACTIVE_MODE;
> +if (!useTemplate)
> +flags = flags | PRNVM_PRESERVE_DISK;

Why do you need to remove this flag to create ct from template? As I remember
it's needed to keep disk images, which you've remove from config.


> +
> +job = PrlVm_RegEx(sdkdom, "", flags);
>  if (PRL_FAILED(waitJob(job)))
>  ret = -1;
>  
-- 
Dmitry Guryanov

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

[libvirt] [PATCH 1/6] process: Allow virProcessPrLimit() to get current limit

[Andrea Bolognani]

The prlimit() function allows both getting and setting limits for
a process; expose the same functionality in our wrapper.

Add the const modifier for new_limit, in accordance with the
prototype for prlimit().
---
 src/util/virprocess.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 4b18903..9b38834 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -725,15 +725,19 @@ int virProcessSetNamespaces(size_t nfdlist,
 
 #if HAVE_PRLIMIT
 static int
-virProcessPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+virProcessPrLimit(pid_t pid,
+  int resource,
+  const struct rlimit *new_limit,
+  struct rlimit *old_limit)
 {
-return prlimit(pid, resource, rlim, NULL);
+return prlimit(pid, resource, new_limit, old_limit);
 }
 #elif HAVE_SETRLIMIT
 static int
 virProcessPrLimit(pid_t pid ATTRIBUTE_UNUSED,
   int resource ATTRIBUTE_UNUSED,
-  struct rlimit *rlim ATTRIBUTE_UNUSED)
+  const struct rlimit *new_limit ATTRIBUTE_UNUSED,
+  struct rlimit *old_limit ATTRIBUTE_UNUSED)
 {
 errno = ENOSYS;
 return -1;
@@ -758,7 +762,7 @@ virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes)
 return -1;
 }
 } else {
-if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, &rlim) < 0) {
+if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, &rlim, NULL) < 0) {
 virReportSystemError(errno,
  _("cannot limit locked memory "
"of process %lld to %llu"),
@@ -803,7 +807,7 @@ virProcessSetMaxProcesses(pid_t pid, unsigned int procs)
 return -1;
 }
 } else {
-if (virProcessPrLimit(pid, RLIMIT_NPROC, &rlim) < 0) {
+if (virProcessPrLimit(pid, RLIMIT_NPROC, &rlim, NULL) < 0) {
 virReportSystemError(errno,
  _("cannot limit number of subprocesses "
"of process %lld to %u"),
@@ -851,7 +855,7 @@ virProcessSetMaxFiles(pid_t pid, unsigned int files)
 return -1;
 }
 } else {
-if (virProcessPrLimit(pid, RLIMIT_NOFILE, &rlim) < 0) {
+if (virProcessPrLimit(pid, RLIMIT_NOFILE, &rlim, NULL) < 0) {
 virReportSystemError(errno,
  _("cannot limit number of open files "
"of process %lld to %u"),
-- 
2.5.0

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

[libvirt] [PATCH 3/6] qemu: Add qemuDomainAdjustMaxMemLock()

[Andrea Bolognani]

This function detects whether a domain needs RLIMIT_MEMLOCK
to be set, and if so, uses an appropriate value.

It also stores the original value inside the virDomainObj for
the domain so that it can be later restored.
---
 src/conf/domain_conf.h |  3 +++
 src/qemu/qemu_domain.c | 50 ++
 src/qemu/qemu_domain.h |  1 +
 3 files changed, 54 insertions(+)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8d43ee6..9e28ac9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2386,6 +2386,9 @@ struct _virDomainObj {
 void (*privateDataFreeFunc)(void *);
 
 int taint;
+
+unsigned long long original_memlock; /* Original RLIMIT_MEMLOCK, zero if no
+  * restore will be required later */
 };
 
 typedef struct _virDomainObjList virDomainObjList;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 18513f9..51a1770 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,6 +40,7 @@
 #include "virstoragefile.h"
 #include "virstring.h"
 #include "virthreadjob.h"
+#include "virprocess.h"
 
 #include "storage/storage_driver.h"
 
@@ -3954,3 +3955,52 @@ qemuDomainRequiresMlock(virDomainDefPtr def)
 
 return false;
 }
+
+/**
+ * qemuDomainAdjustMaxMemLock:
+ *
+ * @vm: domain
+ *
+ * Adjust the memory locking limit for the QEMU process associated to @vm, in
+ * order to comply with VFIO or architecture requirements.
+ *
+ * The limit will not be changed unless doing so is needed; the first time
+ * the limit is changed, the original (default) limit is stored in @vm and
+ * that value will be restored if qemuDomainAdjustMaxMemLock() is called once
+ * memory locking is no longer required.
+ *
+ * Returns: 0 on success, <0 on failure
+ */
+int
+qemuDomainAdjustMaxMemLock(virDomainObjPtr vm)
+{
+unsigned long long bytes = 0;
+int ret = -1;
+
+if (qemuDomainRequiresMlock(vm->def)) {
+/* If this is the first time adjusting the limit, save the current
+ * value so that we can restore it once memory locking is no longer
+ * required */
+if (!vm->original_memlock) {
+if (virProcessGetMaxMemLock(vm->pid, &(vm->original_memlock)) < 0)
+goto out;
+}
+bytes = qemuDomainGetMlockLimitBytes(vm->def);
+} else {
+/* Once memory locking is no longer required, we can restore the
+ * original, usually very low, limit */
+bytes = vm->original_memlock;
+vm->original_memlock = 0;
+}
+
+/* Don't do anything unless we're actually setting a limit */
+if (bytes) {
+if (virProcessSetMaxMemLock(vm->pid, bytes) < 0)
+goto out;
+}
+
+ret = 0;
+
+ out:
+ return ret;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 271dce9..cc64df3 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -486,6 +486,7 @@ int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr 
driver,
 
 unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
 bool qemuDomainRequiresMlock(virDomainDefPtr def);
+int qemuDomainAdjustMaxMemLock(virDomainObjPtr vm);
 
 int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
-- 
2.5.0

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

[libvirt] [PATCH 4/6] qemu: Use qemuDomainAdjustMaxMemLock()

[Andrea Bolognani]

Replace all uses of the qemuDomainRequiresMlock/virProcessSetMaxMemLock
combination with the equivalent qemuDomainAdjustMaxMemLock() call.
---
 src/qemu/qemu_hotplug.c | 40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8804d3d..a5c134a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1276,17 +1276,14 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
 }
 
 /* Temporarily add the hostdev to the domain definition. This is needed
- * because qemuDomainRequiresMlock() and qemuDomainGetMlockLimitBytes()
- * require the hostdev to be already part of the domain definition, but
- * other functions like qemuAssignDeviceHostdevAlias() used below expect
- * it *not* to be there. A better way to handle this would be nice */
+ * because qemuDomainAdjustMaxMemLock() requires the hostdev to be already
+ * part of the domain definition, but other functions like
+ * qemuAssignDeviceHostdevAlias() used below expect it *not* to be there.
+ * A better way to handle this would be nice */
 vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
-if (qemuDomainRequiresMlock(vm->def)) {
-if (virProcessSetMaxMemLock(vm->pid,
-qemuDomainGetMlockLimitBytes(vm->def)) < 
0) {
-vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL;
-goto error;
-}
+if (qemuDomainAdjustMaxMemLock(vm) < 0) {
+vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL;
+goto error;
 }
 vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL;
 
@@ -1772,7 +1769,6 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
 virJSONValuePtr props = NULL;
 virObjectEventPtr event;
 bool fix_balloon = false;
-bool mlock = false;
 int id;
 int ret = -1;
 
@@ -1804,12 +1800,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
-mlock = qemuDomainRequiresMlock(vm->def);
-
-if (mlock &&
-virProcessSetMaxMemLock(vm->pid,
-qemuDomainGetMlockLimitBytes(vm->def)) < 0) {
-mlock = false;
+if (qemuDomainAdjustMaxMemLock(vm) < 0) {
 virJSONValueFree(props);
 goto removedef;
 }
@@ -1870,13 +1861,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
 mem = NULL;
 
 /* reset the mlock limit */
-if (mlock) {
-virErrorPtr err = virSaveLastError();
-ignore_value(virProcessSetMaxMemLock(vm->pid,
- 
qemuDomainGetMlockLimitBytes(vm->def)));
-virSetError(err);
-virFreeError(err);
-}
+virErrorPtr err = virSaveLastError();
+ignore_value(qemuDomainAdjustMaxMemLock(vm));
+virSetError(err);
+virFreeError(err);
 
 goto audit;
 }
@@ -2970,9 +2958,7 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
 virDomainMemoryDefFree(mem);
 
 /* decrease the mlock limit after memory unplug if necessary */
-if (qemuDomainRequiresMlock(vm->def))
-ignore_value(virProcessSetMaxMemLock(vm->pid,
- 
qemuDomainGetMlockLimitBytes(vm->def)));
+ignore_value(qemuDomainAdjustMaxMemLock(vm));
 
 return 0;
 }
-- 
2.5.0

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

[libvirt] [PATCH 6/6] qemu: Replace Mlock with MemLock in function names

[Andrea Bolognani]

MemLock is already used in other modules and, while still an
abbreviation, is not ambiguous.
---
 src/qemu/qemu_command.c |  4 ++--
 src/qemu/qemu_domain.c  | 10 +-
 src/qemu/qemu_domain.h  |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4d00fd9..f6c41ce 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -11095,8 +11095,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 /* In some situations, eg. VFIO passthrough, QEMU might need to lock a
  * significant amount of memory, so we need to set the limit accordingly */
-if (qemuDomainRequiresMlock(def))
-virCommandSetMaxMemLock(cmd, qemuDomainGetMlockLimitBytes(def));
+if (qemuDomainRequiresMemLock(def))
+virCommandSetMaxMemLock(cmd, qemuDomainGetMemLockLimitBytes(def));
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP) &&
 cfg->logTimestamp)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 51a1770..1521213 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3802,7 +3802,7 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
 
 
 /**
- * qemuDomainGetMlockLimitBytes:
+ * qemuDomainGetMemLockLimitBytes:
  *
  * @def: domain definition
  *
@@ -3812,7 +3812,7 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
  * value returned may depend upon the architecture or devices present.
  */
 unsigned long long
-qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
+qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
 {
 unsigned long long memKB;
 
@@ -3933,7 +3933,7 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
  * requirements.
  * */
 bool
-qemuDomainRequiresMlock(virDomainDefPtr def)
+qemuDomainRequiresMemLock(virDomainDefPtr def)
 {
 size_t i;
 
@@ -3977,7 +3977,7 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm)
 unsigned long long bytes = 0;
 int ret = -1;
 
-if (qemuDomainRequiresMlock(vm->def)) {
+if (qemuDomainRequiresMemLock(vm->def)) {
 /* If this is the first time adjusting the limit, save the current
  * value so that we can restore it once memory locking is no longer
  * required */
@@ -3985,7 +3985,7 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm)
 if (virProcessGetMaxMemLock(vm->pid, &(vm->original_memlock)) < 0)
 goto out;
 }
-bytes = qemuDomainGetMlockLimitBytes(vm->def);
+bytes = qemuDomainGetMemLockLimitBytes(vm->def);
 } else {
 /* Once memory locking is no longer required, we can restore the
  * original, usually very low, limit */
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cc64df3..dcb9dc6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -484,8 +484,8 @@ bool qemuDomainMachineHasBuiltinIDE(const virDomainDef 
*def);
 int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
   virDomainObjPtr vm);
 
-unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
-bool qemuDomainRequiresMlock(virDomainDefPtr def);
+unsigned long long qemuDomainGetMemLockLimitBytes(virDomainDefPtr def);
+bool qemuDomainRequiresMemLock(virDomainDefPtr def);
 int qemuDomainAdjustMaxMemLock(virDomainObjPtr vm);
 
 int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
-- 
2.5.0

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

Re: [libvirt] [PATCH 34/34] qemu: cgroup: Don't use priv->ncpupids to iterate domain vCPUs

[Peter Krempa]

On Tue, Nov 24, 2015 at 08:41:53 -0500, John Ferlan wrote:
> 
> 
> On 11/20/2015 10:22 AM, Peter Krempa wrote:
> > Use the proper data structures for the iteration since ncpupids will be
> > made private later.
> > ---
> >  src/qemu/qemu_cgroup.c | 14 --
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> > index d8a2b03..06c20c1 100644
> > --- a/src/qemu/qemu_cgroup.c
> > +++ b/src/qemu/qemu_cgroup.c
> > @@ -800,7 +800,12 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
> >  if (virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0)
> >  goto error;
> > 
> > -for (i = 0; i < priv->nvcpupids; i++) {
> > +for (i = 0; i < virDomainDefGetVCpusMax(vm->def); i++) {
> > +virDomainVCpuInfoPtr vcpu = virDomainDefGetVCpu(vm->def, i);
> > +
> 
> What if !vcpu?  Shouldn't happen, but not checked - trying to consider
> future too.

Actually it really can't and should not ever happen. The setters will
always allocate the array fully so iterating to virDomainDefGetVCpusMax
should always be valid.

Peter


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

[libvirt] [PATCH 0/6] Memory locking limit improvements

[Andrea Bolognani]

As noticed by Peter[1], the memory locking limit for the
QEMU process is increased before assigning a VFIO device to
a guest, but it might not be decreased when returning said
device to the host.

This series fixes this inconsistent behaviour and cleans up
the code a little bit along the way.

The idea is to introduce a new, smarter function called
qemuDomainAdjustMaxMemLock() that does The Right Thing™ and
increases the limit when required, while at the same time
storing the original value. This way, when memory locking
is no longer needed, it can restore it.

I've tested this both on x86 and ppc64, both by removing
devices that were assigned in the domain XML and devices
that I had hotplugged.

Patches 1-2 lay some groundwork by allowing retrieval of
the memory locking limit for a process.

Patches 3-4 add qemuDomainAdjustMaxMemLock() and use it
where appropriate in the existing code.

Patch 5 adds one more use of the function, after a PCI
hostdev has been detached from the guest.

Patch 6 replaces the use of Mlock with MemLock. Suggestions
on how to further improve the names of those functions is
very welcome, this is just a first step in the right
direction.

Cheers.


[1] https://www.redhat.com/archives/libvir-list/2015-November/msg00642.html

Andrea Bolognani (6):
  process: Allow virProcessPrLimit() to get current limit
  process: Add virProcessGetMaxMemLock()
  qemu: Add qemuDomainAdjustMaxMemLock()
  qemu: Use qemuDomainAdjustMaxMemLock()
  qemu: Reduce memlock limit after detaching hostdev
  qemu: Replace Mlock with MemLock in function names

 configure.ac |  2 +-
 src/conf/domain_conf.h   |  3 +++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  |  4 ++--
 src/qemu/qemu_domain.c   | 56 +++---
 src/qemu/qemu_domain.h   |  5 +++--
 src/qemu/qemu_hotplug.c  | 46 --
 src/util/virprocess.c| 58 +++-
 src/util/virprocess.h|  2 ++
 9 files changed, 136 insertions(+), 41 deletions(-)

-- 
2.5.0

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

[libvirt] [PATCH 2/6] process: Add virProcessGetMaxMemLock()

[Andrea Bolognani]

This function can be used to retrieve the current locked memory
limit for a process, so that the setting can be later restored.

Add a configure check for getrlimit(), which we now use.
---
 configure.ac |  2 +-
 src/libvirt_private.syms |  1 +
 src/util/virprocess.c| 42 ++
 src/util/virprocess.h|  2 ++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index f481c50..c2a567f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -279,7 +279,7 @@ AC_CHECK_SIZEOF([long])
 dnl Availability of various common functions (non-fatal if missing),
 dnl and various less common threadsafe functions
 AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
-  getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \
+  getmntent_r getpwuid_r getrlimit getuid kill mmap newlocale posix_fallocate \
   posix_memalign prlimit regexec sched_getaffinity setgroups setns \
   setrlimit symlink sysctlbyname getifaddrs sched_setscheduler])
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7e60d87..e330a16 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2022,6 +2022,7 @@ virPortAllocatorSetUsed;
 virProcessAbort;
 virProcessExitWithStatus;
 virProcessGetAffinity;
+virProcessGetMaxMemLock;
 virProcessGetNamespaces;
 virProcessGetPids;
 virProcessGetStartTime;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 9b38834..b14164a 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -788,6 +788,48 @@ virProcessSetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED, 
unsigned long long bytes)
 }
 #endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_MEMLOCK)) */
 
+#if HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)
+int
+virProcessGetMaxMemLock(pid_t pid,
+unsigned long long *bytes)
+{
+struct rlimit rlim;
+
+if (!bytes)
+return 0;
+
+if (pid == 0) {
+if (getrlimit(RLIMIT_MEMLOCK, &rlim) < 0) {
+virReportSystemError(errno,
+ "%s",
+ _("cannot get locked memory limit"));
+return -1;
+}
+} else {
+if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, NULL, &rlim) < 0) {
+virReportSystemError(errno,
+ _("cannot get locked memory limit "
+   "of process %lld"),
+ (long long int) pid);
+return -1;
+}
+}
+
+/* virProcessSetMaxMemLock() sets both rlim_cur and rlim_max to the
+ * same value, so we can retrieve just rlim_max here */
+*bytes = rlim.rlim_max;
+
+return 0;
+}
+#else /* ! (HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)) */
+int
+virProcessGetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED,
+unsigned long long *bytes)
+{
+virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
+return -1;
+}
+#endif /* ! (HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)) */
 
 #if HAVE_SETRLIMIT && defined(RLIMIT_NPROC)
 int
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 1768009..a7a1fe9 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -76,6 +76,8 @@ int virProcessSetMaxMemLock(pid_t pid, unsigned long long 
bytes);
 int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
 int virProcessSetMaxFiles(pid_t pid, unsigned int files);
 
+int virProcessGetMaxMemLock(pid_t pid, unsigned long long *bytes);
+
 /* Callback to run code within the mount namespace tied to the given
  * pid.  This function must use only async-signal-safe functions, as
  * it gets run after a fork of a multi-threaded process.  The return
-- 
2.5.0

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

[libvirt] [PATCH 5/6] qemu: Reduce memlock limit after detaching hostdev

[Andrea Bolognani]

We increase the limit before plugging in a PCI hostdev or a memory
module because some memory might need to be locked due to eg. VFIO.

Of course we should do the opposite after unplugging a device: this
was already the case for memory modules, but not for hostdevs.
---
 src/qemu/qemu_hotplug.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a5c134a..04baeff 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3070,6 +3070,12 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
 networkReleaseActualDevice(vm->def, net);
 virDomainNetDefFree(net);
 }
+
+/* QEMU might no longer need to lock as much memory, eg. we just detached
+ * a VFIO device, so adjust the limit here */
+if (qemuDomainAdjustMaxMemLock(vm) < 0)
+VIR_WARN("Failed to adjust locked memory limit");
+
 ret = 0;
 
  cleanup:
-- 
2.5.0

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

Re: [libvirt] [PATCH 34/34] qemu: cgroup: Don't use priv->ncpupids to iterate domain vCPUs

[John Ferlan]

On 11/20/2015 10:22 AM, Peter Krempa wrote:
> Use the proper data structures for the iteration since ncpupids will be
> made private later.
> ---
>  src/qemu/qemu_cgroup.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index d8a2b03..06c20c1 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -800,7 +800,12 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
>  if (virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0)
>  goto error;
> 
> -for (i = 0; i < priv->nvcpupids; i++) {
> +for (i = 0; i < virDomainDefGetVCpusMax(vm->def); i++) {
> +virDomainVCpuInfoPtr vcpu = virDomainDefGetVCpu(vm->def, i);
> +

What if !vcpu?  Shouldn't happen, but not checked - trying to consider
future too.

> +if (!vcpu->online)
> +continue;
> +
>  if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
> false, &cgroup_temp) < 0 ||
>  virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 ||
> @@ -1016,7 +1021,12 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
>  &mem_mask, -1) < 0)
>  goto cleanup;
> 
> -for (i = 0; i < priv->nvcpupids; i++) {
> +for (i = 0; i < virDomainDefGetVCpusMax(def); i++) {
> +virDomainVCpuInfoPtr vcpu = virDomainDefGetVCpu(def, i);
> +

Same here

ACK w/ adjustments... That reminds me - patch 33 will have the same issue.

John
> +if (!vcpu->online)
> +continue;
> +
>  virCgroupFree(&cgroup_vcpu);
>  if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
> true, &cgroup_vcpu) < 0)
> 

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

Re: [libvirt] [PATCH 05/34] conf: Use local copy of maxvcpus in virDomainVcpuParse

[Peter Krempa]

On Mon, Nov 23, 2015 at 07:04:43 -0500, John Ferlan wrote:
> 
> 
> On 11/20/2015 10:21 AM, Peter Krempa wrote:
> > Use the local variable rather than getting it all the time from the
> > struct. This will simplify further refactors.
> > ---
> >  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 0ac7dbf..3c8a926 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -14664,13 +14664,13 @@ virDomainVcpuParse(virDomainDefPtr def,
> >  goto cleanup;
> >  }
> > 
> > -def->vcpus = def->maxvcpus;
> > +def->vcpus = maxvcpus;
> 
> There is no local maxvcpus (yet) and this breaks git bisect.
> 
> ACK 1-5 with this fixed

Indeed, this patch was moved a little bit too far. After putting it
behind "conf: Replace writes to def->maxvcpus with accessor" it compiles
just fine. I've pushed 1-4 and I'll push this after the mentioned patch
after fixing it.

Thanks

Peter


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

Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 02:24:22PM +0100, Jiri Denemark wrote:
> On Tue, Nov 17, 2015 at 16:00:40 +0800, Qiaowei Ren wrote:
> > The series mainly adds Intel CMT feature support into libvirt. CMT is
> > new introduced PQos (Platform Qos) feature to monitor the usage of
> > cache by applications running on the platform.
> > 
> > Currently CMT patches has been merged into Linux kernel mainline.
> > The CMT implementation in Linux kernel is based on perf mechanism and
> > there is no support for perf in libvirt, and so this series firstly
> > add perf support into libvirt, including two public API and a set of
> > util interfaces. And based on these APIs and interfaces, thie series
> > implements CMT perf event support.
> > 
> > TODO:
> > 1. This series relys on keeping an open file descriptor for the guest.
> > We should add one patch to call sys_perf_event_open again iff libvirtd
> > is restarted.
> 
> Yes, we should reenable perf events when reconnecting to running
> domains. Will we need to remember what events were enabled (in domain
> status XML) or is it something we can read back from the kernel?

We need to record it somewhere. I guess this raises the question of
whether we should hide it in status XML, or persist the list of
desired perf events in the real domain XML, so we can have the option
of also having them enabled immediately at startup, without requiring
separate API call.

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 0/8] Add perf and Intel CMT feature support

[Jiri Denemark]

On Tue, Nov 17, 2015 at 16:00:40 +0800, Qiaowei Ren wrote:
> The series mainly adds Intel CMT feature support into libvirt. CMT is
> new introduced PQos (Platform Qos) feature to monitor the usage of
> cache by applications running on the platform.
> 
> Currently CMT patches has been merged into Linux kernel mainline.
> The CMT implementation in Linux kernel is based on perf mechanism and
> there is no support for perf in libvirt, and so this series firstly
> add perf support into libvirt, including two public API and a set of
> util interfaces. And based on these APIs and interfaces, thie series
> implements CMT perf event support.
> 
> TODO:
> 1. This series relys on keeping an open file descriptor for the guest.
> We should add one patch to call sys_perf_event_open again iff libvirtd
> is restarted.

Yes, we should reenable perf events when reconnecting to running
domains. Will we need to remember what events were enabled (in domain
status XML) or is it something we can read back from the kernel?

Jirka

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

[libvirt] [PATCH] virtlockd: fix misc memory leaks and other bugs

[Daniel P. Berrange]

Fix memory leaks, failure to restore umask and missing man
page docs.

Signed-off-by: Daniel P. Berrange 
---
 src/locking/lock_daemon.c| 6 +-
 src/locking/virtlockd.pod.in | 7 ++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 3dc47c1..a5a40fe 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -118,6 +118,7 @@ virLockDaemonFree(virLockDaemonPtr lockd)
 if (!lockd)
 return;
 
+virMutexDestroy(&lockd->lock);
 virObjectUnref(lockd->dmn);
 virHashFree(lockd->lockspaces);
 virLockSpaceFree(lockd->defaultLockspace);
@@ -410,6 +411,7 @@ virLockDaemonUnixSocketPaths(bool privileged,
 
 old_umask = umask(077);
 if (virFileMakePath(rundir) < 0) {
+VIR_FREE(rundir);
 umask(old_umask);
 goto error;
 }
@@ -516,6 +518,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
 
 old_umask = umask(077);
 if (virFileMakePath(logdir) < 0) {
+VIR_FREE(logdir);
 umask(old_umask);
 goto error;
 }
@@ -1187,7 +1190,7 @@ int main(int argc, char **argv) {
 int c;
 char *tmp;
 
-c = getopt_long(argc, argv, "ldf:p:t:vVh", opts, &optidx);
+c = getopt_long(argc, argv, "df:p:t:vVh", opts, &optidx);
 
 if (c == -1)
 break;
@@ -1321,6 +1324,7 @@ int main(int argc, char **argv) {
 VIR_ERROR(_("unable to create rundir %s: %s"), run_dir,
   virStrerror(errno, ebuf, sizeof(ebuf)));
 ret = VIR_LOCK_DAEMON_ERR_RUNDIR;
+umask(old_umask);
 goto cleanup;
 }
 umask(old_umask);
diff --git a/src/locking/virtlockd.pod.in b/src/locking/virtlockd.pod.in
index 99612aa..661473c 100644
--- a/src/locking/virtlockd.pod.in
+++ b/src/locking/virtlockd.pod.in
@@ -4,7 +4,7 @@ virtlockd - libvirt lock management daemon
 
 =head1 SYNOPSIS
 
-B [ -dv ] [ -f config_file ] [ -p pid_file ]
+B [ -dvV ] [ -t timeout] [ -f config_file ] [ -p pid_file ]
 
 B --version
 
@@ -38,6 +38,11 @@ Run as a daemon and write PID file.
 
 Use this configuration file, overriding the default value.
 
+=item B<-t, --timeout> I
+
+Automatically shutdown after I have elapsed with
+no active client or lock.
+
 =item B<-p, --pid-file> I
 
 Use this name for the PID file, overriding the default value.
-- 
2.5.0

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

Re: [libvirt] [PATCH 7/8] virsh: extend domstats command

[Jiri Denemark]

On Tue, Nov 17, 2015 at 16:00:47 +0800, Qiaowei Ren wrote:
> This patch extend domstats command to match extended
> virDomainListGetStats API in previous patch.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  tools/virsh-domain-monitor.c | 7 +++
>  tools/virsh.pod  | 7 +--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index d4e500b..33df079 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -2016,6 +2016,10 @@ static const vshCmdOptDef opts_domstats[] = {
>   .type = VSH_OT_BOOL,
>   .help = N_("report domain block device statistics"),
>  },
> +{.name = "perf",
> + .type = VSH_OT_BOOL,
> + .help = N_("report domain perf event statistics"),
> +},
>  {.name = "list-active",
>   .type = VSH_OT_BOOL,
>   .help = N_("list only active domains"),
> @@ -2127,6 +2131,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
>  if (vshCommandOptBool(cmd, "block"))
>  stats |= VIR_DOMAIN_STATS_BLOCK;
>  
> +if (vshCommandOptBool(cmd, "perf"))
> +stats |= VIR_DOMAIN_STATS_PERF;
> +
>  if (vshCommandOptBool(cmd, "list-active"))
>  flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE;
>  
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 21ae4a3..935d017 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -845,8 +845,8 @@ I for disk snapshots) will accept either 
> target
>  or unique source names printed by this command.
>  
>  =item B [I<--raw>] [I<--enforce>] [I<--backing>] [I<--state>]
> -[I<--cpu-total>] [I<--balloon>] [I<--vcpu>] [I<--interface>] [I<--block>]
> -[[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>]
> +[I<--cpu-total>] [I<--balloon>] [I<--vcpu>] [I<--interface>] [I<--perf>]
> +[I<--block>] [[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>]
>  [I<--list-transient>] [I<--list-running>] [I<--list-paused>]
>  [I<--list-shutoff>] [I<--list-other>]] | [I ...]
>  
> @@ -899,6 +899,9 @@ I<--interface> returns:
>  "net..tx.errs" - number of transmission errors,
>  "net..tx.drop" - number of transmit packets dropped
>  
> +I<--perf> returns:
> +"perf.cache" - the cache usage in Byte currently used

I think this feature would deserve to be documented a bit more. It would
be nice to provide more details about what perf events are. And there
are a lot of caches so describing what cache usage is measured by
perf.cache would be useful too.

Jirka

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

[libvirt] [PATCH] logging: inhibit virtlogd shutdown while log files are open

[Daniel P. Berrange]

The virtlogd daemon is launched with a 30 second timeout for
unprivileged users. Unfortunately the timeout is only inhibited
while RPC clients are connected, and they only connect for a
short while to open the log file descriptor. We need to hold
an inhibition for as long as the log file descriptor itself
is open.

Signed-off-by: Daniel P. Berrange 
---
 src/logging/log_daemon.c  | 19 +--
 src/logging/log_handler.c | 33 +++--
 src/logging/log_handler.h | 11 +--
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 7a1afec..3fda9ca 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -124,6 +124,17 @@ virLogDaemonFree(virLogDaemonPtr logd)
 }
 
 
+static void
+virLogDaemonInhibitor(bool inhibit, void *opaque)
+{
+virLogDaemonPtr daemon = opaque;
+
+if (inhibit)
+virNetDaemonAddShutdownInhibition(daemon->dmn);
+else
+virNetDaemonRemoveShutdownInhibition(daemon->dmn);
+}
+
 static virLogDaemonPtr
 virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged)
 {
@@ -152,7 +163,9 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool 
privileged)
 virNetDaemonAddServer(logd->dmn, logd->srv) < 0)
 goto error;
 
-if (!(logd->handler = virLogHandlerNew(privileged)))
+if (!(logd->handler = virLogHandlerNew(privileged,
+   virLogDaemonInhibitor,
+   logd)))
 goto error;
 
 return logd;
@@ -210,7 +223,9 @@ virLogDaemonNewPostExecRestart(virJSONValuePtr object, bool 
privileged)
 }
 
 if (!(logd->handler = virLogHandlerNewPostExecRestart(child,
-  privileged)))
+  privileged,
+  
virLogDaemonInhibitor,
+  logd)))
 goto error;
 
 return logd;
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index 2acbca7..a4f0395 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -59,6 +59,9 @@ struct _virLogHandler {
 bool privileged;
 virLogHandlerLogFilePtr *files;
 size_t nfiles;
+
+virLogHandlerShutdownInhibitor inhibitor;
+void *opaque;
 };
 
 static virClassPtr virLogHandlerClass;
@@ -165,13 +168,16 @@ virLogHandlerDomainLogFileEvent(int watch,
 return;
 
  error:
+handler->inhibitor(false, handler->opaque);
 virLogHandlerLogFileClose(handler, logfile);
 virObjectUnlock(handler);
 }
 
 
 virLogHandlerPtr
-virLogHandlerNew(bool privileged)
+virLogHandlerNew(bool privileged,
+ virLogHandlerShutdownInhibitor inhibitor,
+ void *opaque)
 {
 virLogHandlerPtr handler;
 
@@ -182,6 +188,8 @@ virLogHandlerNew(bool privileged)
 goto error;
 
 handler->privileged = privileged;
+handler->inhibitor = inhibitor;
+handler->opaque = opaque;
 
 return handler;
 
@@ -191,7 +199,8 @@ virLogHandlerNew(bool privileged)
 
 
 static virLogHandlerLogFilePtr
-virLogHandlerLogFilePostExecRestart(virJSONValuePtr object)
+virLogHandlerLogFilePostExecRestart(virLogHandlerPtr handler,
+virJSONValuePtr object)
 {
 virLogHandlerLogFilePtr file;
 const char *path;
@@ -199,6 +208,8 @@ virLogHandlerLogFilePostExecRestart(virJSONValuePtr object)
 if (VIR_ALLOC(file) < 0)
 return NULL;
 
+handler->inhibitor(true, handler->opaque);
+
 if ((path = virJSONValueObjectGetString(object, "path")) == NULL) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing file path in JSON document"));
@@ -226,6 +237,7 @@ virLogHandlerLogFilePostExecRestart(virJSONValuePtr object)
 return file;
 
  error:
+handler->inhibitor(false, handler->opaque);
 virLogHandlerLogFileFree(file);
 return NULL;
 }
@@ -233,14 +245,18 @@ virLogHandlerLogFilePostExecRestart(virJSONValuePtr 
object)
 
 virLogHandlerPtr
 virLogHandlerNewPostExecRestart(virJSONValuePtr object,
-bool privileged)
+bool privileged,
+virLogHandlerShutdownInhibitor inhibitor,
+void *opaque)
 {
 virLogHandlerPtr handler;
 virJSONValuePtr files;
 ssize_t n;
 size_t i;
 
-if (!(handler = virLogHandlerNew(privileged)))
+if (!(handler = virLogHandlerNew(privileged,
+ inhibitor,
+ opaque)))
 return NULL;
 
 if (!(files = virJSONValueObjectGet(object, "files"))) {
@@ -259,7 +275,7 @@ virLogHandlerNewPostExecRestart(virJSONValuePtr object,
 virLogHandlerLogFilePtr file;
 virJSONValuePtr child = virJSONValueArrayGet(files, 

Re: [libvirt] [perl PATCH] Post-release version bump to 1.3.0

[Pavel Hrdina]

On Tue, Nov 24, 2015 at 12:51:47PM +, Daniel P. Berrange wrote:
> On Tue, Nov 24, 2015 at 01:41:33PM +0100, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  Makefile.PL | 2 +-
> >  README  | 2 +-
> >  lib/Sys/Virt.pm | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> Need to add a dummy entry in Changes eg just put in a dummy date and
> changelog item
> 
> [quote] 2015-12-00
>   1.3.0
> 
>- XXX
> [/quote]
> 
> otherwise the unit tests will complain
> 
> ACK with that change added.

Thanks, I've updated the patch and pushed it.

Pavel

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

Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

[Jiri Denemark]

On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> This patch implement the internal driver API for perf event into
> qemu driver.
> 
> In addition, this patch extend virDomainListGetStats API to get
> the statistics for perf event. To do so, we add a
> 'VIR_DOMAIN_STATS_PERF' enum to causes reporting of all previously
> enabled perf events.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  include/libvirt/libvirt-domain.h |   1 +
>  src/qemu/qemu_domain.h   |   3 +
>  src/qemu/qemu_driver.c   | 181 
> +++
>  src/qemu/qemu_process.c  |   6 ++
>  4 files changed, 191 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 69965e6..5e74b29 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1771,6 +1771,7 @@ typedef enum {
>  VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
>  VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info 
> */
>  VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
> +VIR_DOMAIN_STATS_PERF = (1 << 6), /* return domain perf event info */
>  } virDomainStatsTypes;
>  
>  typedef enum {
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 4be998c..0348d4a 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -26,6 +26,7 @@
>  
>  # include "virthread.h"
>  # include "vircgroup.h"
> +# include "virperf.h"
>  # include "domain_addr.h"
>  # include "domain_conf.h"
>  # include "snapshot_conf.h"
> @@ -190,6 +191,8 @@ struct _qemuDomainObjPrivate {
>  
>  virCgroupPtr cgroup;
>  
> +virPerfPtr perf;
> +
>  virCond unplugFinished; /* signals that unpluggingDevice was unplugged */
>  const char *unpluggingDevice; /* alias of the device that is being 
> unplugged */
>  char **qemuDevices; /* NULL-terminated list of devices aliases known to 
> QEMU */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 92a9961..5ad4d79 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -98,6 +98,7 @@
>  #include "virhostdev.h"
>  #include "domain_capabilities.h"
>  #include "vircgroup.h"
> +#include "virperf.h"
>  #include "virnuma.h"
>  #include "dirname.h"
>  #include "network/bridge_driver.h"
> @@ -113,6 +114,8 @@ VIR_LOG_INIT("qemu.qemu_driver");
>  
>  #define QEMU_NB_NUMA_PARAM 2
>  
> +#define QEMU_NB_PERF_PARAM VIR_PERF_EVENT_LAST
> +

VIR_PERF_EVENT_LAST is good enough, no need to define another symbol for
it.

>  #define QEMU_SCHED_MIN_PERIOD  1000LL
>  #define QEMU_SCHED_MAX_PERIOD   100LL
>  #define QEMU_SCHED_MIN_QUOTA   1000LL
> @@ -10244,6 +10247,106 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>  }
>  
>  static int
> +qemuDomainSetPerfEvents(virDomainPtr dom,
> +virTypedParameterPtr params,
> +int nparams)
> +{
> +size_t i;
> +virDomainObjPtr vm = NULL;
> +qemuDomainObjPrivatePtr priv;
> +int ret = -1;
> +bool enabled;
> +
> +if (virTypedParamsValidate(params, nparams,
> +   VIR_DOMAIN_PERF_CMT,
> +   VIR_TYPED_PARAM_BOOLEAN,
> +   NULL) < 0)
> +return -1;

Use virTypedParamsCheck and define the data for it in virperf.h so that
you don't need to change the code here if new event type is added.

> +
> +if (!(vm = qemuDomObjFromDomain(dom)))
> +return -1;
> +
> +priv = vm->privateData;
> +
> +if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +for (i = 0; i < nparams; i++) {
> +virTypedParameterPtr param = ¶ms[i];
> +enabled = params[i].value.b;
> +
> +if (STREQ(param->field, VIR_DOMAIN_PERF_CMT)) {

Just use the appropriate FromString to convert the param field into the
perf type enum and call virPerfEvent{Disable,Enable} depending on the
value. Thus no change will be needed here if you add more perf events.

> +if (!enabled && virPerfEventDisable(VIR_PERF_EVENT_CMT, 
> priv->perf)) {
> +virReportError(VIR_ERR_NO_SUPPORT,
> +   _("can't disable perf event: %s"),
> +   VIR_DOMAIN_PERF_CMT);
> +goto cleanup;
> +}
> +if (enabled && virPerfCmtEnable(vm->pid, priv->perf)) {
> +virReportError(VIR_ERR_NO_SUPPORT,
> +   _("can't enable perf event: %s"),
> +   VIR_DOMAIN_PERF_CMT);
> +goto cleanup;
> +}

Both virPerfEventDisable and virPerfEventEnable should have reported the
error, no need to mask it with another one.

> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +virDomainObjEndAPI(&vm);
> +return ret;
> +}
> +
> +static int
> +qemuDomainGetPerfEven

Re: [libvirt] [PATCH 33/34] qemu: driver: Refactor qemuDomainHelperGetVcpus

[John Ferlan]

On 11/20/2015 10:22 AM, Peter Krempa wrote:
> Change some of the control structures and switch to using the new vcpu
> structure.
> ---
>  src/qemu/qemu_driver.c | 77 
> --
>  1 file changed, 43 insertions(+), 34 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c659328..b9f8e72 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1422,11 +1422,17 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
> *lastCpu, long *vm_rss,
> 
> 
>  static int
> -qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int 
> maxinfo,
> - unsigned char *cpumaps, int maplen)
> +qemuDomainHelperGetVcpus(virDomainObjPtr vm,
> + virVcpuInfoPtr info,
> + int maxinfo,
> + unsigned char *cpumaps,
> + int maplen)
>  {
> -size_t i, v;
> -qemuDomainObjPrivatePtr priv = vm->privateData;
> +size_t ncpuinfo = 0;
> +size_t i;
> +
> +if (maxinfo == 0)
> +return 0;
> 
>  if (!qemuDomainHasVCpuPids(vm)) {
>  virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -1434,43 +1440,46 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, 
> virVcpuInfoPtr info, int maxinfo,
>  return -1;
>  }
> 
> -/* Clamp to actual number of vcpus */
> -if (maxinfo > priv->nvcpupids)
> -maxinfo = priv->nvcpupids;
> -
> -if (maxinfo >= 1) {
> -if (info != NULL) {
> -memset(info, 0, sizeof(*info) * maxinfo);
> -for (i = 0; i < maxinfo; i++) {
> -info[i].number = i;
> -info[i].state = VIR_VCPU_RUNNING;
> -
> -if (qemuGetProcessInfo(&(info[i].cpuTime),
> -   &(info[i].cpu),
> -   NULL,
> -   vm->pid,
> -   qemuDomainGetVCpuPid(vm, i)) < 0) {
> -virReportSystemError(errno, "%s",
> - _("cannot get vCPU placement & pCPU 
> time"));
> -return -1;
> -}
> +if (info)
> +memset(info, 0, sizeof(*info) * maxinfo);
> +
> +if (cpumaps)
> +memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo);
> +
> +for (i = 0; i < virDomainDefGetVCpusMax(vm->def) && ncpuinfo < maxinfo; 
> i++) {

This line is longer than 80 cols.

> +virDomainVCpuInfoPtr vcpu = virDomainDefGetVCpu(vm->def, i);
> +pid_t vcpupid = qemuDomainGetVCpuPid(vm, i);
> +
> +if (!vcpu->online)
> +continue;

So if the goal is to eventually allow vcpu 0 & 2 of 4 vcpu's to be
online, then this algorithm will need a slight adjustment.

Of course there's also the what if 'vcpupid == 0' that hasn't been
checked here (comments from patch 32).

I "believe" what needs to be done is change the [i] below to [ncpuinfo]
- that way the info & cpumaps would be returned with only the
ONLINE/RUNNING vCPU's and 'info[]' won't have gaps which won't be
accessible if a "2" is returned...  I think the same holds true for the
VIR_GET_CPUMAP


ACK with some adjustments.

John

> +
> +if (info) {
> +info[i].number = i;
> +info[i].state = VIR_VCPU_RUNNING;
> +
> +if (qemuGetProcessInfo(&(info[i].cpuTime), &(info[i].cpu), NULL,
> +   vm->pid, vcpupid) < 0) {
> +virReportSystemError(errno, "%s",
> + _("cannot get vCPU placement & pCPU 
> time"));
> +return -1;
>  }
>  }
> 
> -if (cpumaps != NULL) {
> -for (v = 0; v < maxinfo; v++) {
> -unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
> -virBitmapPtr map = NULL;
> +if (cpumaps) {
> +unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, i);
> +virBitmapPtr map = NULL;
> 
> -if (!(map = virProcessGetAffinity(qemuDomainGetVCpuPid(vm, 
> v
> -return -1;
> +if (!(map = virProcessGetAffinity(vcpupid)))
> +return -1;
> 
> -virBitmapToDataBuf(map, cpumap, maplen);
> -virBitmapFree(map);
> -}
> +virBitmapToDataBuf(map, cpumap, maplen);
> +virBitmapFree(map);
>  }
> +
> +ncpuinfo++;
>  }
> -return maxinfo;
> +
> +return ncpuinfo;
>  }
> 
> 

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

Re: [libvirt] [PATCH 5/8] perf: implement a set of util functions for perf event

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 01:44:53PM +0100, Jiri Denemark wrote:
> On Tue, Nov 24, 2015 at 13:23:49 +0100, Jiri Denemark wrote:
> > On Tue, Nov 17, 2015 at 16:00:45 +0800, Qiaowei Ren wrote:
> > > This patch implement a set of interfaces for perf event. Based on
> > > these interfaces, we can implement internal driver API for perf,
> > > and get the results of perf conuter you care about.
> > > 
> > > Signed-off-by: Qiaowei Ren 
> ...
> > > diff --git a/src/util/virperf.c b/src/util/virperf.c
> > > new file mode 100644
> > > index 000..9c71858
> > > --- /dev/null
> > > +++ b/src/util/virperf.c
> ...
> > > +
> > > +int
> > > +virPerfCmtEnable(pid_t pid,
> > > + virPerfPtr perf)
> > 
> > I think 'perf' should always be the first parameter of all virPerf
> > functions, since it is the object all the functions operate on.
> > 
> > >From a caller perspective, I think it would be a bit better to create a
> > generic int virPerfEventEnable(perf, type, pid) entry point, and make
> > virPerfCmtEnable static. Unless each type requires different input data.
> 
> Actually, you can just pass vm to virPerfEventEnable, that should given
> enough input data to any perf event.

The virPerfEventEnable code lives in util which is supposed to be
isolated from any conf classes, so passing 'vm' is not appropriate

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 v2] Post-release version bump to 1.3.0

[Pavel Hrdina]

On Tue, Nov 24, 2015 at 12:49:11PM +, Daniel P. Berrange wrote:
> On Tue, Nov 24, 2015 at 01:28:14PM +0100, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  configure.ac | 2 +-
> >  src/libxl/libxl_driver.c | 4 ++--
> >  src/vz/vz_driver.c   | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> ACK

Thanks, pushed now

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

Re: [libvirt] [perl PATCH] Post-release version bump to 1.3.0

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 01:41:33PM +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  Makefile.PL | 2 +-
>  README  | 2 +-
>  lib/Sys/Virt.pm | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Need to add a dummy entry in Changes eg just put in a dummy date and
changelog item

[quote] 2015-12-00
  1.3.0

   - XXX
[/quote]

otherwise the unit tests will complain

ACK with that change added.

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 v2] Post-release version bump to 1.3.0

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 01:28:14PM +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  configure.ac | 2 +-
>  src/libxl/libxl_driver.c | 4 ++--
>  src/vz/vz_driver.c   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

ACK


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 5/8] perf: implement a set of util functions for perf event

[Jiri Denemark]

On Tue, Nov 24, 2015 at 13:23:49 +0100, Jiri Denemark wrote:
> On Tue, Nov 17, 2015 at 16:00:45 +0800, Qiaowei Ren wrote:
> > This patch implement a set of interfaces for perf event. Based on
> > these interfaces, we can implement internal driver API for perf,
> > and get the results of perf conuter you care about.
> > 
> > Signed-off-by: Qiaowei Ren 
...
> > diff --git a/src/util/virperf.c b/src/util/virperf.c
> > new file mode 100644
> > index 000..9c71858
> > --- /dev/null
> > +++ b/src/util/virperf.c
...
> > +
> > +int
> > +virPerfCmtEnable(pid_t pid,
> > + virPerfPtr perf)
> 
> I think 'perf' should always be the first parameter of all virPerf
> functions, since it is the object all the functions operate on.
> 
> >From a caller perspective, I think it would be a bit better to create a
> generic int virPerfEventEnable(perf, type, pid) entry point, and make
> virPerfCmtEnable static. Unless each type requires different input data.

Actually, you can just pass vm to virPerfEventEnable, that should given
enough input data to any perf event.

Jirka

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

[libvirt] [perl PATCH] Post-release version bump to 1.3.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 Makefile.PL | 2 +-
 README  | 2 +-
 lib/Sys/Virt.pm | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile.PL b/Makefile.PL
index 709be83..4ae538f 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -3,7 +3,7 @@ use ExtUtils::MakeMaker;
 # See lib/ExtUtils/MakeMaker.pm for details of how to influence
 # the contents of the Makefile that is written.
 
-my $libvirtver = "1.2.21";
+my $libvirtver = "1.3.0";
 my $stat = system "pkg-config --atleast-version=$libvirtver libvirt";
 die "cannot run pkg-config to check libvirt version" if $stat == -1;
 die "libvirt >= $libvirtver is required\n" unless $stat == 0;
diff --git a/README b/README
index 757d652..a942d66 100644
--- a/README
+++ b/README
@@ -7,6 +7,6 @@ further details on libvirt consult its website 
http://libvirt.org/
 The only pre-requisite for this module is libvirt itself. For
 installation instructions, consult the INSTALL file.
 
-The current minimum required version of libvirt is 1.2.21
+The current minimum required version of libvirt is 1.3.0
 
 -- End
diff --git a/lib/Sys/Virt.pm b/lib/Sys/Virt.pm
index 0371d08..9e7c6d9 100644
--- a/lib/Sys/Virt.pm
+++ b/lib/Sys/Virt.pm
@@ -78,7 +78,7 @@ use Sys::Virt::NWFilter;
 use Sys::Virt::DomainSnapshot;
 use Sys::Virt::Stream;
 
-our $VERSION = '1.2.21';
+our $VERSION = '1.3.0';
 require XSLoader;
 XSLoader::load('Sys::Virt', $VERSION);
 
-- 
2.6.3

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

[libvirt] [python PATCH] Post-release version bump to 1.3.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---

Pushed

 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 61fa369..04e3de4 100755
--- a/setup.py
+++ b/setup.py
@@ -311,7 +311,7 @@ class my_clean(clean):
 _c_modules, _py_modules = get_module_lists()
 
 setup(name = 'libvirt-python',
-  version = '1.2.21',
+  version = '1.3.0',
   url = 'http://www.libvirt.org',
   maintainer = 'Libvirt Maintainers',
   maintainer_email = 'libvir-list@redhat.com',
-- 
2.6.3

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

Re: [libvirt] [PATCH] qemu: pass the asyncJob to qemuProcessStartCPUs

[Ján Tomko]

On Tue, Nov 24, 2015 at 01:33:41PM +0100, Peter Krempa wrote:
> On Tue, Nov 24, 2015 at 13:25:05 +0100, Ján Tomko wrote:
> > Now that new domains are started inside a QEMU_ASYNC_JOB_START job,
> > we need to pass it down to qemuProcessStartCPUs too.
> > 
> > This removes the warning:
> > qemuDomainObjEnterMonitorInternal:1750 : This thread seems to be the
> > async job owner; entering monitor without asking for a nested job is
> > dangerous
> > 
> > Introduced by commit 04c721f, before that this code path was only
> > executed with QEMU_ASYNC_JOB_NONE.
> > 
> > (This code is not executed on migration, because qemuMigrationPrepareAny
> >  sets the VIR_QEMU_PROCESS_START_PAUSED flag.)
> > ---
> >  src/qemu/qemu_process.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> ACK

Thanks, 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] qemu: pass the asyncJob to qemuProcessStartCPUs

[Peter Krempa]

On Tue, Nov 24, 2015 at 13:25:05 +0100, Ján Tomko wrote:
> Now that new domains are started inside a QEMU_ASYNC_JOB_START job,
> we need to pass it down to qemuProcessStartCPUs too.
> 
> This removes the warning:
> qemuDomainObjEnterMonitorInternal:1750 : This thread seems to be the
> async job owner; entering monitor without asking for a nested job is
> dangerous
> 
> Introduced by commit 04c721f, before that this code path was only
> executed with QEMU_ASYNC_JOB_NONE.
> 
> (This code is not executed on migration, because qemuMigrationPrepareAny
>  sets the VIR_QEMU_PROCESS_START_PAUSED flag.)
> ---
>  src/qemu/qemu_process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK


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

[libvirt] [PATCHv5 8/9] tests: add tests for multiple panic devices

[Dmitry Andreev]

---
 .../qemuxml2argv-panic-double.args | 21 
 .../qemuxml2argvdata/qemuxml2argv-panic-double.xml | 28 ++
 tests/qemuxml2argvtest.c   |  2 ++
 tests/qemuxml2xmltest.c|  1 +
 4 files changed, 52 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-double.args 
b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.args
new file mode 100644
index 000..0d3a385
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.args
@@ -0,0 +1,21 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-cpu qemu32,hv_crash \
+-m 214 \
+-smp 6 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait \
+-boot n \
+-usb \
+-device pvpanic,ioport=1285
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml
new file mode 100644
index 000..aadb758
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml
@@ -0,0 +1,28 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  6
+  
+hvm
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+
+
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a15305d..ea16c98 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1633,6 +1633,8 @@ mymain(void)
 
 DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,
 QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+DO_TEST("panic-double", QEMU_CAPS_DEVICE_PANIC,
+QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 
 DO_TEST("panic-no-address", QEMU_CAPS_DEVICE_PANIC,
 QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0e43ee9..fd58331 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -594,6 +594,7 @@ mymain(void)
 DO_TEST_DIFFERENT("panic");
 DO_TEST("panic-isa");
 DO_TEST("panic-pseries");
+DO_TEST("panic-double");
 DO_TEST("panic-no-address");
 
 DO_TEST_DIFFERENT("disk-backing-chains");
-- 
1.8.3.1

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

[libvirt] [PATCHv5 3/9] tests: add tests for the new panic device attribute - 'model'

[Dmitry Andreev]

---
v5: tests was moved from another patch

 .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 +
 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml  | 31 ++
 .../qemuxml2argv-panic-pseries.xml | 30 +
 tests/qemuxml2xmltest.c|  3 +++
 4 files changed, 89 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml
new file mode 100644
index 000..9f0edbb
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml
@@ -0,0 +1,25 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  6
+  
+hvm
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml
new file mode 100644
index 000..b9595a8
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml
@@ -0,0 +1,31 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml
new file mode 100644
index 000..8fcd644
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml
@@ -0,0 +1,30 @@
+
+  QEMUGuest1
+  1ccfd97d-5eb4-478a-bbe6-88d254c16db7
+  524288
+  524288
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-ppc64
+
+
+
+  
+  
+
+
+  
+  
+
+
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index cbd4d0d..fbb46d6 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -375,6 +375,7 @@ mymain(void)
 
 DO_TEST("hyperv");
 DO_TEST("hyperv-off");
+DO_TEST("hyperv-panic");
 
 DO_TEST("kvm-features");
 DO_TEST("kvm-features-off");
@@ -591,6 +592,8 @@ mymain(void)
 DO_TEST("pcihole64-q35");
 
 DO_TEST("panic");
+DO_TEST("panic-isa");
+DO_TEST("panic-pseries");
 DO_TEST("panic-no-address");
 
 DO_TEST_DIFFERENT("disk-backing-chains");
-- 
1.8.3.1

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

[libvirt] [PATCH v2] Post-release version bump to 1.3.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 configure.ac | 2 +-
 src/libxl/libxl_driver.c | 4 ++--
 src/vz/vz_driver.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index f481c50..4b7c9ed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General 
Public
 dnl License along with this library.  If not, see
 dnl .
 
-AC_INIT([libvirt], [1.2.22], [libvir-list@redhat.com], [], 
[http://libvirt.org])
+AC_INIT([libvirt], [1.3.0], [libvir-list@redhat.com], [], [http://libvirt.org])
 AC_CONFIG_SRCDIR([src/libvirt.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d77a0e4..35d7fae 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5421,8 +5421,8 @@ static virHypervisorDriver libxlHypervisorDriver = {
 #endif
 .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
 .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
-.domainMemoryStats = libxlDomainMemoryStats, /* 1.2.22 */
-.domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.22 */
+.domainMemoryStats = libxlDomainMemoryStats, /* 1.3.0 */
+.domainGetCPUStats = libxlDomainGetCPUStats, /* 1.3.0 */
 .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
 .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 
0.9.0 */
 .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 39f58a4..ea1090a 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1493,7 +1493,7 @@ static virHypervisorDriver vzDriver = {
 .domainShutdown = vzDomainShutdown, /* 0.10.0 */
 .domainCreate = vzDomainCreate,/* 0.10.0 */
 .domainCreateWithFlags = vzDomainCreateWithFlags, /* 1.2.10 */
-.domainReboot = vzDomainReboot, /* 1.2.22 */
+.domainReboot = vzDomainReboot, /* 1.3.0 */
 .domainDefineXML = vzDomainDefineXML,  /* 0.10.0 */
 .domainDefineXMLFlags = vzDomainDefineXMLFlags, /* 1.2.12 */
 .domainUndefine = vzDomainUndefine, /* 1.2.10 */
-- 
2.6.3

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

[libvirt] [PATCHv5 7/9] Allow multiple panic devices

[Dmitry Andreev]

'model' attribute was added to a panic device but only one panic
device is allowed. This patch changes panic device presence
from 'optional' to 'zeroOrMore'.
---
v5: part of the code was moved out from this commit

 docs/formatdomain.html.in |  1 +
 docs/schemas/domaincommon.rng |  4 ++--
 src/conf/domain_conf.c| 53 ---
 src/conf/domain_conf.h|  4 +++-
 src/qemu/qemu_command.c   | 50 
 src/qemu/qemu_domain.c| 21 +
 6 files changed, 77 insertions(+), 56 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 32b196d..06aec4b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6152,6 +6152,7 @@ qemu-kvm -net nic,model=? /dev/null
 
   ...
   
+
 
   

 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d21650..7e7fd58 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4044,9 +4044,9 @@
 
   
 
-
+
   
-
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b8738af..ef322f5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2538,7 +2538,9 @@ void virDomainDefFree(virDomainDefPtr def)
 
 virDomainTPMDefFree(def->tpm);
 
-virDomainPanicDefFree(def->panic);
+for (i = 0; i < def->npanics; i++)
+virDomainPanicDefFree(def->panics[i]);
+VIR_FREE(def->panics);
 
 VIR_FREE(def->idmap.uidmap);
 VIR_FREE(def->idmap.gidmap);
@@ -3617,10 +3619,10 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
 if (cb(def, &device, &def->tpm->info, opaque) < 0)
 return -1;
 }
-if (def->panic) {
-device.type = VIR_DOMAIN_DEVICE_PANIC;
-device.data.panic = def->panic;
-if (cb(def, &device, &def->panic->info, opaque) < 0)
+device.type = VIR_DOMAIN_DEVICE_PANIC;
+for (i = 0; i < def->npanics; i++) {
+device.data.panic = def->panics[i];
+if (cb(def, &device, &def->panics[i]->info, opaque) < 0)
 return -1;
 }
 
@@ -16412,23 +16414,19 @@ virDomainDefParseXML(xmlDocPtr xml,
 VIR_FREE(nodes);
 
 /* analysis of the panic devices */
-def->panic = NULL;
 if ((n = virXPathNodeSet("./devices/panic", ctxt, &nodes)) < 0)
 goto error;
-if (n > 1) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("only a single panic device is supported"));
+if (n && VIR_ALLOC_N(def->panics, n) < 0)
 goto error;
-}
-if (n > 0) {
+for (i = 0; i < n; i++) {
 virDomainPanicDefPtr panic =
-virDomainPanicDefParseXML(nodes[0]);
+virDomainPanicDefParseXML(nodes[i]);
 if (!panic)
 goto error;
 
-def->panic = panic;
-VIR_FREE(nodes);
+def->panics[def->npanics++] = panic;
 }
+VIR_FREE(nodes);
 
 /* analysis of the shmem devices */
 if ((n = virXPathNodeSet("./devices/shmem", ctxt, &nodes)) < 0)
@@ -17641,17 +17639,6 @@ static bool
 virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
 virDomainPanicDefPtr dst)
 {
-if (!src && !dst)
-return true;
-
-if (!src || !dst) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain panic device count '%d' "
- "does not match source count '%d'"),
-   src ? 1 : 0, dst ? 1 : 0);
-return false;
-}
-
 if (src->model != dst->model) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target panic model '%s' does not match source '%s'"),
@@ -18138,8 +18125,16 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
 if (!virDomainRNGDefCheckABIStability(src->rngs[i], dst->rngs[i]))
 goto error;
 
-if (!virDomainPanicDefCheckABIStability(src->panic, dst->panic))
+if (src->npanics != dst->npanics) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target domain panic device count %zu "
+ "does not match source %zu"), dst->npanics, 
src->npanics);
 goto error;
+}
+
+for (i = 0; i < src->npanics; i++)
+if (!virDomainPanicDefCheckABIStability(src->panics[i], 
dst->panics[i]))
+goto error;
 
 if (src->nshmems != dst->nshmems) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -22457,9 +22452,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 if (def->nvram)
 virDomainNVRAMDefFormat(buf, def->nvram, flags);
 
-if (def->panic &&
-virDomainPanicDefFormat(buf, def->panic) < 0)
-goto error;
+for (n = 0; n < def->npanics; n++)
+if (virDomainPanicDefFor

[libvirt] [PATCHv5 9/9] conf: reject multiple panic devices of same model

[Dmitry Andreev]

Only one panic device per model is allowed.
---
v5: minor code fixes

 src/conf/domain_conf.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ef322f5..c30f420 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3747,6 +3747,28 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr 
def)
 return ret;
 }
 
+static int
+virDomainDefRejectDuplicatePanics(virDomainDefPtr def)
+{
+bool exists[VIR_DOMAIN_PANIC_MODEL_LAST];
+size_t i;
+
+for (i = 0; i < VIR_DOMAIN_PANIC_MODEL_LAST; i++)
+ exists[i] = false;
+
+for (i = 0; i < def->npanics; i++) {
+virDomainPanicModel model = def->panics[i]->model;
+if (exists[model]) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Multiple panic devices with model '%s'"),
+   virDomainPanicModelTypeToString(model));
+return -1;
+}
+exists[model] = true;
+}
+
+return 0;
+}
 
 /**
  * virDomainDefMetadataSanitize:
@@ -3976,6 +3998,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 if (virDomainDefRejectDuplicateControllers(def) < 0)
 return -1;
 
+if (virDomainDefRejectDuplicatePanics(def) < 0)
+return -1;
+
 /* verify settings of guest timers */
 for (i = 0; i < def->clock.ntimers; i++) {
 virDomainTimerDefPtr timer = def->clock.timers[i];
-- 
1.8.3.1

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

[libvirt] [PATCHv5 2/9] conf: add 'model' attribute for panic device with values isa, pseries, hyperv

[Dmitry Andreev]

Libvirt already has two types of panic devices - pvpanic and pSeries firmware.
This patch introduces the 'model' attribute and a new type of panic device.

'isa' model is for ISA pvpanic device.
'pseries' model is a default value for pSeries guests.
'hyperv' model is the new type. It's used for Hyper-V crash.

Schema and docs are updated for the new attribute.
---
v5: minor fixes
 
 docs/formatdomain.html.in | 18 --
 docs/schemas/domaincommon.rng |  9 +
 src/conf/domain_conf.c| 34 +-
 src/conf/domain_conf.h| 11 +++
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e5e0167..32b196d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6152,19 +6152,33 @@ qemu-kvm -net nic,model=? /dev/null
 
   ...
   
-
+
   

 
   
   ...
 
   
+model
+  
+
+  The optional model attribute specifies what type
+  of panic device is provided. The panic model used when this attribute
+  is missing depends on the hypervisor and guest arch.
+
+
+  'isa' — for ISA pvpanic device
+  'pseries' — default and valid only for pSeries guests.
+  'hyperv' — for Hyper-V crash CPU feature.
+Since 1.3.0, QEMU and KVM only
+
+  
 address
 
   
 address of panic. The default ioport is 0x505. Most users
 don't need to specify an address, and doing so is forbidden
-altogether for pSeries guests.
+altogether for pseries and hyperv models.
   
 
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 994face..9d21650 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5361,6 +5361,15 @@
   
 
   
+
+  
+isa
+pseries
+hyperv
+  
+
+  
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a14dd77..b8738af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -525,6 +525,12 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, 
VIR_DOMAIN_WATCHDOG_ACTION_LAST,
   "none",
   "inject-nmi")
 
+VIR_ENUM_IMPL(virDomainPanicModel, VIR_DOMAIN_PANIC_MODEL_LAST,
+  "default",
+  "isa",
+  "pseries",
+  "hyperv")
+
 VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
   "vga",
   "cirrus",
@@ -10197,6 +10203,7 @@ static virDomainPanicDefPtr
 virDomainPanicDefParseXML(xmlNodePtr node)
 {
 virDomainPanicDefPtr panic;
+char *model = NULL;
 
 if (VIR_ALLOC(panic) < 0)
 return NULL;
@@ -10204,10 +10211,22 @@ virDomainPanicDefParseXML(xmlNodePtr node)
 if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, 0) < 0)
 goto error;
 
+model = virXMLPropString(node, "model");
+if (model != NULL &&
+(panic->model = virDomainPanicModelTypeFromString(model)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown panic model '%s'"), model);
+goto error;
+}
+
+ cleanup:
+VIR_FREE(model);
 return panic;
+
  error:
 virDomainPanicDefFree(panic);
-return NULL;
+panic = NULL;
+goto cleanup;
 }
 
 /* Parse the XML definition for an input device */
@@ -17633,6 +17652,14 @@ 
virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
 return false;
 }
 
+if (src->model != dst->model) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target panic model '%s' does not match source '%s'"),
+   virDomainPanicModelTypeToString(dst->model),
+   virDomainPanicModelTypeToString(src->model));
+return false;
+}
+
 return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
 }
 
@@ -20619,6 +20646,11 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
 int indent = virBufferGetIndent(buf, false);
 
 virBufferAddLit(buf, "model)
+virBufferAsprintf(buf, " model='%s'",
+  virDomainPanicModelTypeToString(def->model));
+
 virBufferAdjustIndent(&childrenBuf, indent + 2);
 if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
 return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8d43ee6..11d891f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2045,7 +2045,17 @@ struct _virDomainIdMapDef {
 };
 
 
+typedef enum {
+VIR_DOMAIN_PANIC_MODEL_DEFAULT,
+VIR_DOMAIN_PANIC_MODEL_ISA,
+VIR_DOMAIN_PANIC_MODEL_PSERIES,
+VIR_DOMAIN_PANIC_MODEL_HYPERV,
+
+VIR_DOMAIN_PANIC_MODEL_LAST
+} virDomainPanicModel;
+
 struct _virDomainPanicDe

[libvirt] [PATCHv5 0/9] Hyper-v crash feature support

[Dmitry Andreev]

A new Hyper-V cpu feature 'hv_crash' was added to QEMU. The feature
will become available in v2.5.0.

What is changed in v5:
* minor code fixes
* code was moved between patches
* patch sequence changed

Dmitry Andreev (9):
  conf: refactor code for checking ABI stability of panic device
  conf: add 'model' attribute for panic device with values isa, pseries,
hyperv
  tests: add tests for the new panic device attribute - 'model'
  qemu: add support for hv_crash feature as a panic device
  tests: rework tests for panic devices
  tests: add tests for the new 'hyperv' panic device model
  Allow multiple panic devices
  tests: add tests for multiple panic devices
  conf: reject multiple panic devices of same model

 docs/formatdomain.html.in  |  19 +++-
 docs/schemas/domaincommon.rng  |  13 ++-
 src/conf/domain_conf.c | 123 +
 src/conf/domain_conf.h |  15 ++-
 src/qemu/qemu_command.c|  80 --
 src/qemu/qemu_domain.c |  30 -
 tests/qemuargv2xmltest.c   |   1 +
 .../qemuxml2argv-hyperv-panic.args |  21 
 .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml |  25 +
 .../qemuxml2argv-panic-double.args |  21 
 .../qemuxml2argvdata/qemuxml2argv-panic-double.xml |  28 +
 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml  |  31 ++
 .../qemuxml2argv-panic-no-address.xml  |   2 +-
 .../qemuxml2argv-panic-pseries.xml |  30 +
 .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |   2 +-
 .../qemuxml2argv-pseries-nvram.xml |   2 +-
 tests/qemuxml2argvtest.c   |   3 +
 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml  |  31 ++
 .../qemuxml2xmlout-pseries-panic-missing.xml   |   2 +-
 .../qemuxml2xmlout-pseries-panic-no-address.xml|  30 +
 tests/qemuxml2xmltest.c|   8 +-
 21 files changed, 448 insertions(+), 69 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml

-- 
1.8.3.1

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

[libvirt] [PATCHv5 6/9] tests: add tests for the new 'hyperv' panic device model

[Dmitry Andreev]

---
 tests/qemuargv2xmltest.c|  1 +
 .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.args | 21 +
 tests/qemuxml2argvtest.c|  1 +
 3 files changed, 23 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args

diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 34f3e5c..7759a09 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -261,6 +261,7 @@ mymain(void)
 DO_TEST("smp");
 
 DO_TEST("hyperv");
+DO_TEST("hyperv-panic");
 
 DO_TEST("kvm-features");
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args
new file mode 100644
index 000..a9f13e0
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args
@@ -0,0 +1,21 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-cpu qemu32,hv_crash \
+-m 214 \
+-smp 6 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-monitor unix:/tmp/test-monitor,server,nowait \
+-boot n \
+-usb \
+-net none \
+-serial none \
+-parallel none
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index dc8654e..a15305d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -692,6 +692,7 @@ mymain(void)
 
 DO_TEST("hyperv", NONE);
 DO_TEST("hyperv-off", NONE);
+DO_TEST("hyperv-panic", NONE);
 
 DO_TEST("kvm-features", NONE);
 DO_TEST("kvm-features-off", NONE);
-- 
1.8.3.1

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

[libvirt] [PATCHv5 4/9] qemu: add support for hv_crash feature as a panic device

[Dmitry Andreev]

Panic device type used depends on 'model' attribute.

If no model is specified then device type depends on hypervisor
and guest arch. 'pseries' model is used for pSeries guest and
'isa' model is used in other cases.

XML:

  


QEMU command line:
qemu -cpu ,hv_crash
---
v5:
* autoselection of panic device model in post parse was moved
  from another patch
* ARCH_IS_X86 check for 'hyperv' panic device was added

 src/qemu/qemu_command.c | 60 -
 src/qemu/qemu_domain.c  |  9 
 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4d00fd9..bb6d5fe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7577,6 +7577,16 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver,
 }
 }
 
+if (def->panic &&
+def->panic->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) {
+if (!have_cpu) {
+virBufferAdd(&buf, default_model, -1);
+have_cpu = true;
+}
+
+virBufferAddLit(&buf, ",hv_crash");
+}
+
 if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
 if (!have_cpu) {
 virBufferAdd(&buf, default_model, -1);
@@ -11050,17 +11060,45 @@ qemuBuildCommandLine(virConnectPtr conn,
 }
 
 if (def->panic) {
-if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, 
"pseries")) {
-/* For pSeries guests, the firmware provides the same
- * functionality as the pvpanic device. The address
+switch ((virDomainPanicModel) def->panic->model) {
+case VIR_DOMAIN_PANIC_MODEL_HYPERV:
+/* Panic with model 'hyperv' is not a device, it should
+ * be configured in cpu commandline. The address
  * cannot be configured by the user */
+if (!ARCH_IS_X86(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("only i686 and x86_64 architectures are 
support "
+ "panic device of model 'hyperv'"));
+goto error;
+}
 if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("setting the panic device address is not "
- "supported for pSeries guests"));
+ "supported for model 'hyperv'"));
 goto error;
 }
-} else {
+break;
+
+case VIR_DOMAIN_PANIC_MODEL_PSERIES:
+if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, 
"pseries")) {
+/* For pSeries guests, the firmware provides the same
+ * functionality as the pvpanic device. The address
+ * cannot be configured by the user */
+if (def->panic->info.type != 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("setting the panic device address is not "
+ "supported for model 'pseries'"));
+goto error;
+}
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("only pSeries guests support panic device "
+ "of model 'pseries'"));
+goto error;
+}
+break;
+
+case VIR_DOMAIN_PANIC_MODEL_ISA:
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the QEMU binary does not support the "
@@ -11085,6 +11123,11 @@ qemuBuildCommandLine(virConnectPtr conn,
  "with ISA address type"));
 goto error;
 }
+
+/* default model value was changed before in post parse */
+case VIR_DOMAIN_PANIC_MODEL_DEFAULT:
+case VIR_DOMAIN_PANIC_MODEL_LAST:
+break;
 }
 }
 
@@ -12436,6 +12479,13 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
 if (virCPUDefAddFeature(cpu, feature, policy) < 0)
 goto cleanup;
 }
+} else if (STREQ(tokens[i], "hv_crash")) {
+virDomainPanicDefPtr panic;
+if (VIR_ALLOC(panic) < 0)
+goto cleanup;
+
+panic->model = VIR_DOMAIN_PANIC_MODEL_HYPERV;
+dom->panic = panic;
 } else if (STRPREFIX(tokens[i], "hv_")) {
 const char *token = tokens[i] + 3; /* "hv_" */
 const char *feature, *value;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 18513f9..689abc2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1350,6 +1350,15 @@ qemuDomainDeviceDefPo

[libvirt] [PATCHv5 5/9] tests: rework tests for panic devices

[Dmitry Andreev]

Panic device model will be selected automatically if it is not
chosen by the user. Tests should cover this case.
---
 .../qemuxml2argv-panic-no-address.xml  |  2 +-
 .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
 .../qemuxml2argv-pseries-nvram.xml |  2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml  | 31 ++
 .../qemuxml2xmlout-pseries-panic-missing.xml   |  2 +-
 .../qemuxml2xmlout-pseries-panic-no-address.xml| 30 +
 tests/qemuxml2xmltest.c|  4 +--
 7 files changed, 67 insertions(+), 6 deletions(-)
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
index 79f8a1e..f3f3fbb 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
@@ -24,6 +24,6 @@
 
 
 
-
+
   
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
index 3a96209..39f4a1f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
@@ -37,6 +37,6 @@
   
 
 
-
+
   
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml
index 619186a..2da2832 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml
@@ -20,6 +20,6 @@
 
   
 
-
+
   
 
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml
new file mode 100644
index 000..b9595a8
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml
@@ -0,0 +1,31 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
index 9312975..8fcd644 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
@@ -25,6 +25,6 @@
   
 
 
-
+
   
 
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml
new file mode 100644
index 000..8fcd644
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml
@@ -0,0 +1,30 @@
+
+  QEMUGuest1
+  1ccfd97d-5eb4-478a-bbe6-88d254c16db7
+  524288
+  524288
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-ppc64
+
+
+
+  
+  
+
+
+  
+  
+
+
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index fbb46d6..0e43ee9 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -535,7 +535,7 @@ mymain(void)
 
 DO_TEST("pseries-nvram");
 DO_TEST_DIFFERENT("pseries-panic-missing");
-DO_TEST("pseries-panic-no-address");
+DO_TEST_DIFFERENT("pseries-panic-no-address");
 
 /* These tests generate different XML */
 DO_TEST_DIFFERENT("balloon-device-auto");
@@ -591,7 +591,7 @@ mymain(void)
 DO_TEST("pcihole64-none");
 DO_TEST("pcihole64-q35");
 
-DO_TEST("panic");
+DO_TEST_DIFFERENT("panic");
 DO_TEST("panic-isa");
 DO_TEST("panic-pseries");
 DO_TEST("panic-no-address");
-- 
1.8.3.1

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

[libvirt] [PATCHv5 1/9] conf: refactor code for checking ABI stability of panic device

[Dmitry Andreev]

---
v5: this code was moved from another patch
 src/conf/domain_conf.c | 21 ++---
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ac7dbf..a14dd77 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17619,7 +17619,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 }
 
 static bool
-virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
+virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
 virDomainPanicDefPtr dst)
 {
 if (!src && !dst)
@@ -17695,13 +17695,6 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr 
src,
 }
 
 static bool
-virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
-   virDomainPanicDefPtr dst)
-{
-return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
-}
-
-static bool
 virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src,
 virDomainMemoryDefPtr dst)
 {
@@ -18118,7 +18111,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
 if (!virDomainRNGDefCheckABIStability(src->rngs[i], dst->rngs[i]))
 goto error;
 
-if (!virDomainPanicCheckABIStability(src->panic, dst->panic))
+if (!virDomainPanicDefCheckABIStability(src->panic, dst->panic))
 goto error;
 
 if (src->nshmems != dst->nshmems) {
@@ -18143,16 +18136,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
 goto error;
 }
 
-if (src->panic && dst->panic) {
-if (!virDomainPanicDefCheckABIStability(src->panic, dst->panic))
-goto error;
-} else if (src->panic || dst->panic) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Either both target and source domains or none of "
- "them must have PANIC device present"));
-goto error;
-}
-
 if (src->nmems != dst->nmems) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target domain memory device count %zu "
-- 
1.8.3.1

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

Re: [libvirt] [PATCH 32/34] qemu: Add helper to retrieve vCPU pid

[John Ferlan]

On 11/20/2015 10:22 AM, Peter Krempa wrote:
> Instead of directly accessing the array add a helper to do this.
> ---
>  src/qemu/qemu_cgroup.c  |  3 ++-
>  src/qemu/qemu_domain.c  | 20 
>  src/qemu/qemu_domain.h  |  1 +
>  src/qemu/qemu_driver.c  |  7 ---
>  src/qemu/qemu_process.c |  5 ++---
>  5 files changed, 29 insertions(+), 7 deletions(-)
> 

I believe technically it's a "tid" and not a "pid", but since 1/2 the
code calls it one way and the other calls it a different way it's a coin
flip on whether it...  Theoretically different than vm->pid too
(although I am still trying to recall why vcpupids[0] was being compared
to vm->pid)

Couple of other places according to cscope that still reference vcpupids[]:

qemuDomainObjPrivateXMLFormat
qemuDomainObjPrivateXMLParse

What about the innocent bystanders? IOW: What gets called with a now
potentially "0" for pid if the passed vcpu is out of bounds?  Where 0 is
self/current process for some I believe. Not that this is any less worse
than what theoretically could happen with some out of range fetch, but
the question is more towards should we not make the call then?  If the
goal is to make the code better, then perhaps the "error condition"
should be checked.

virCgroupAddTask
qemuGetProcessInfo
virProcessGetAffinity
virProcessSetAffinity
qemuProcessSetSchedParams

> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 56c2e90..d8a2b03 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -1023,7 +1023,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
>  goto cleanup;
> 
>  /* move the thread for vcpu to sub dir */
> -if (virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]) < 0)
> +if (virCgroupAddTask(cgroup_vcpu,
> + qemuDomainGetVCpuPid(vm, i)) < 0)
>  goto cleanup;
> 
>  if (period || quota) {
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8a45825..be1f2b4 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3971,3 +3971,23 @@ qemuDomainHasVCpuPids(virDomainObjPtr vm)
> 
>  return priv->nvcpupids > 0;
>  }
> +
> +
> +/**
> + * qemuDomainGetVCpuPid:
> + * @vm: domain object
> + * @vcpu: cpu id
> + *
> + * Returns the vCPU pid. If @vcpu is offline or out of range 0 is returned.
> + */
> +pid_t
> +qemuDomainGetVCpuPid(virDomainObjPtr vm,
> + unsigned int vcpu)

Would prefer "VCPU" or "Vcpu"

An ACK would be conditional on your thoughts regarding usage of tid vs.
pid and how much error checking to do.  I'm assuming right now that
you're setting something up for the next batch of changes.

John
> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +if (vcpu >= priv->nvcpupids)
> +return 0;
> +
> +return priv->vcpupids[vcpu];
> +}
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 7f2eca1..c1aad61 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -492,5 +492,6 @@ int qemuDomainDefValidateMemoryHotplug(const virDomainDef 
> *def,
> const virDomainMemoryDef *mem);
> 
>  bool qemuDomainHasVCpuPids(virDomainObjPtr vm);
> +pid_t qemuDomainGetVCpuPid(virDomainObjPtr vm, unsigned int vcpu);
> 
>  #endif /* __QEMU_DOMAIN_H__ */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4b7452c..c659328 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1449,7 +1449,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, 
> virVcpuInfoPtr info, int maxinfo,
> &(info[i].cpu),
> NULL,
> vm->pid,
> -   priv->vcpupids[i]) < 0) {
> +   qemuDomainGetVCpuPid(vm, i)) < 0) {
>  virReportSystemError(errno, "%s",
>   _("cannot get vCPU placement & pCPU 
> time"));
>  return -1;
> @@ -1462,7 +1462,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, 
> virVcpuInfoPtr info, int maxinfo,
>  unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
>  virBitmapPtr map = NULL;
> 
> -if (!(map = virProcessGetAffinity(priv->vcpupids[v])))
> +if (!(map = virProcessGetAffinity(qemuDomainGetVCpuPid(vm, 
> v
>  return -1;
> 
>  virBitmapToDataBuf(map, cpumap, maplen);
> @@ -5156,7 +5156,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>  goto endjob;
>  }
>  } else {
> -if (virProcessSetAffinity(priv->vcpupids[vcpu], pcpumap) < 0) {
> +if (virProcessSetAffinity(qemuDomainGetVCpuPid(vm, vcpu),
> +  pcpumap) < 0) {
>  virReportError(VIR_ERR_SYSTEM_ERROR,
>   

[libvirt] [PATCH] qemu: pass the asyncJob to qemuProcessStartCPUs

[Ján Tomko]

Now that new domains are started inside a QEMU_ASYNC_JOB_START job,
we need to pass it down to qemuProcessStartCPUs too.

This removes the warning:
qemuDomainObjEnterMonitorInternal:1750 : This thread seems to be the
async job owner; entering monitor without asking for a nested job is
dangerous

Introduced by commit 04c721f, before that this code path was only
executed with QEMU_ASYNC_JOB_NONE.

(This code is not executed on migration, because qemuMigrationPrepareAny
 sets the VIR_QEMU_PROCESS_START_PAUSED flag.)
---
 src/qemu/qemu_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2192ad8..68542e7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5013,15 +5013,15 @@ int qemuProcessStart(virConnectPtr conn,
 goto error;
 
 if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) {
 VIR_DEBUG("Starting domain CPUs");
 /* Allow the CPUS to start executing */
 if (qemuProcessStartCPUs(driver, vm, conn,
  VIR_DOMAIN_RUNNING_BOOTED,
- QEMU_ASYNC_JOB_NONE) < 0) {
+ asyncJob) < 0) {
 if (virGetLastError() == NULL)
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("resume operation failed"));
 goto error;
 }
 } else {
 virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
-- 
2.4.6

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

Re: [libvirt] [PATCH 2/2] nodedev: report maxCount for virtual_functions capability

[Pavel Hrdina]

On Mon, Nov 23, 2015 at 04:50:14PM -0500, Laine Stump wrote:
> A PCI device may have the capability to setup virtual functions (VFs)
> but have them currently all disabled. Prior to this patch, if that was
> the case the the node device XML for the device wouldn't report any

s/the the/the/

> virtual_functions capability.
> 
> With this patch, if a file called "sriov_totalvfs" is found in the
> device's sysfs directory, its contents will be interpreted as a
> decimal number, and that value will be reported as "maxCount" in a
> capability element of the device's XML, e.g.:
> 
>
> 
> This will be reported regardless of whether or not any VFs are
> currently enabled for the device.
> 
> NB: sriov_numvfs (the number of VFs currently active) is also
> available in sysfs, but that value is implied by the number of items
> in the list that is inside the capability element, so there is no
> reason to explicitly provide it as an attribute.
> 
> sriov_totalvfs and sriov_numvfs are available in kernels at least as far
> back as the 2.6.32 that is in RHEL6.7, but in the case that they
> simply aren't there, libvirt will behave as it did prior to this patch
> - no maxCount will be displayed, and the virtual_functions capability
> will be absent from the device's XML when 0 VFs are enabled.
> ---
>  src/network/bridge_driver.c   |  5 +++--
>  src/node_device/node_device_linux_sysfs.c |  7 +--
>  src/util/virnetdev.c  |  9 ++---
>  src/util/virnetdev.h  |  5 +++--
>  src/util/virpci.c | 30 ++
>  src/util/virpci.h |  5 +++--
>  6 files changed, 46 insertions(+), 15 deletions(-)
> 

ACK

Pavel

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

Re: [libvirt] [PATCH 5/8] perf: implement a set of util functions for perf event

[Jiri Denemark]

On Tue, Nov 17, 2015 at 16:00:45 +0800, Qiaowei Ren wrote:
> This patch implement a set of interfaces for perf event. Based on
> these interfaces, we can implement internal driver API for perf,
> and get the results of perf conuter you care about.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  include/libvirt/virterror.h |   2 +
>  src/Makefile.am |   1 +
>  src/libvirt_private.syms|   8 ++
>  src/util/virerror.c |   2 +
>  src/util/virperf.c  | 222 
> 
>  src/util/virperf.h  |  60 
>  6 files changed, 295 insertions(+)
>  create mode 100644 src/util/virperf.c
>  create mode 100644 src/util/virperf.h
> 
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index f716cb9..59dd8e7 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -128,6 +128,8 @@ typedef enum {
>  VIR_FROM_THREAD = 61,   /* Error from thread utils */
>  VIR_FROM_ADMIN = 62,/* Error from admin backend */
>  

Extra empty line.

> +VIR_FROM_PERF = 63, /* Error from perf */
> +
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_ERR_DOMAIN_LAST
>  # endif
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 99b4993..afc6cf0 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -98,6 +98,7 @@ UTIL_SOURCES =  
> \
>   util/virauthconfig.c util/virauthconfig.h   \
>   util/virbitmap.c util/virbitmap.h   \
>   util/virbuffer.c util/virbuffer.h   \
> + util/virperf.c util/virperf.h   \
>   util/vircgroup.c util/vircgroup.h util/vircgrouppriv.h  \
>   util/virclosecallbacks.c util/virclosecallbacks.h   
> \
>   util/vircommand.c util/vircommand.h util/vircommandpriv.h \
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a835f18..f0e2bd5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1988,6 +1988,14 @@ virPCIGetVirtualFunctions;
>  virPCIIsVirtualFunction;
>  
>  
> +# util/vircgroup.h

s/vircgroup/virperf/

> +virPerfInitialize;
> +virPerfFree;
> +virPerfEventOfType;
> +virPerfCmtEnable;
> +virPerfEventDisable;
> +
> +
>  # util/virpidfile.h
>  virPidFileAcquire;
>  virPidFileAcquirePath;
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index 6dc05f4..12a5218 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -134,6 +134,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
>"Polkit", /* 60 */
>"Thread jobs",
>"Admin Interface",
> +

Extra empty line.

> +  "Perf",
>  )
>  
>  
> diff --git a/src/util/virperf.c b/src/util/virperf.c
> new file mode 100644
> index 000..9c71858
> --- /dev/null
> +++ b/src/util/virperf.c
> @@ -0,0 +1,222 @@
> +/*
> + * virperf.c: methods for managing perf events
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * .
> + *
> + * Authors:
> + *  Ren Qiaowei 
> + */
> +#include 
> +
> +#include 
> +#if defined HAVE_SYS_SYSCALL_H
> +# include 
> +#endif
> +
> +#include "virperf.h"
> +#include "viralloc.h"
> +#include "virerror.h"
> +#include "virlog.h"
> +#include "virfile.h"
> +#include "virstring.h"
> +#include "virtypedparam.h"
> +
> +VIR_LOG_INIT("util.perf");
> +
> +#define VIR_FROM_THIS VIR_FROM_PERF
> +
> +#if defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)
> +# define VIR_PERF_SUPPORTED
> +#endif
> +
> +VIR_ENUM_IMPL(virPerfEvent, VIR_PERF_EVENT_LAST,
> +  "cmt");
> +
> +
> +#ifdef VIR_PERF_SUPPORTED

Why don't you use defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)
directly here?

> +
> +#include 

This should be "# include ".

> +
> +int
> +virPerfInitialize(virPerfPtr *perf)

We usually create such functions with virPerfPtr virPerfNew(void)
prototype. You can then use it

if (!(perf = virPerfNew()))
goto error;

> +{
> +size_t i;
> +
> +if (VIR_ALLOC((*perf)) < 0)
> +return -1;
> +
> +for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
> +(*perf)->events[i].type = i;
> +(*perf)->events[i].fd = -1;
> +(*perf)->events[i].enabled = false;
> +}
> +
> +return 0;
> +}

> +
> +void
> +virP

Re: [libvirt] [PATCH] Post-release version bump to 1.3.0

[Daniel P. Berrange]

On Tue, Nov 24, 2015 at 07:01:03AM -0500, John Ferlan wrote:
> 
> 
> On 11/24/2015 06:52 AM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina 
> > ---
> > 
> > We've dropped support for old QEMU and there was a vote to change version to
> > 1.3.0, let's make it official.
> > 
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> 
> I was wondering who was going to roll the dice  I agree we should
> change, but we also need to change a few other places such as
> vz_driver.c and libxl_driver.c with API listing 1.2.22 in comments
> (which affects the hvsupport.html.in)

Yep, there's a few updates in libvirt

$ git grep '1\.2\.22'
configure.ac:AC_INIT([libvirt], [1.2.22], [libvir-list@redhat.com], [], 
[http://libvirt.org])
src/libxl/libxl_driver.c:.domainMemoryStats = libxlDomainMemoryStats, /* 
1.2.22 */
src/libxl/libxl_driver.c:.domainGetCPUStats = libxlDomainGetCPUStats, /* 
1.2.22 */
src/vz/vz_driver.c:.domainReboot = vzDomainReboot, /* 1.2.22 */

and also the Perl and Python bindings need updating to match.

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 1/2] conf: support reporting maxCount attribute for virtual_functions cap

[Pavel Hrdina]

On Mon, Nov 23, 2015 at 04:50:13PM -0500, Laine Stump wrote:
> Report the maximum possible number of VFs for an SRIOV PF, like this:
> 
>
>   ...
>
> 
> I've just discovered that the virtual_functions and physical_functions
> capabilities are not supported in the virNodeDeviceParse functions,
> only in virNodeDeviceFormat (I suppose because they are only reported,
> not set from XML). This should probably be remedied, but is less
> immediately useful than the current patch.
> ---
>  docs/formatnode.html.in | 11 ++-
>  src/conf/node_device_conf.c | 32 
>  src/conf/node_device_conf.h |  1 +
>  3 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
> index ed00af5..79e2448 100644
> --- a/docs/formatnode.html.in
> +++ b/docs/formatnode.html.in
> @@ -108,7 +108,16 @@
>  the type is virtual_functions, then this
>  device is an SRIOV PF, and the capability element will
>  have a list of address subelements, one
> -for each VF on this PF.
> +for each VF on this PF. If the host system supports
> +reporting it (via the "sriov_maxvfs" file in the
> +device's sysfs directory) the capability element will
> +also have an attribute named maxCount
> +which is the maximum number of SRIOV VFs supported by
> +this device, which could be higher than the number of
> +VFs that are curently active since
> +1.2.22; in this case, even if there are

The new version will be 1.3.0

> +currently no active VFs the virtual_functions
> +capabililty will still be shown.
>
>numa
>

[...]

ACK with that change.

Pavel

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

Re: [libvirt] [PATCH] Post-release version bump to 1.3.0

[John Ferlan]

On 11/24/2015 06:52 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
> 
> We've dropped support for old QEMU and there was a vote to change version to
> 1.3.0, let's make it official.
> 
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

I was wondering who was going to roll the dice  I agree we should
change, but we also need to change a few other places such as
vz_driver.c and libxl_driver.c with API listing 1.2.22 in comments
(which affects the hvsupport.html.in)

John
> diff --git a/configure.ac b/configure.ac
> index f481c50..4b7c9ed 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser 
> General Public
>  dnl License along with this library.  If not, see
>  dnl .
>  
> -AC_INIT([libvirt], [1.2.22], [libvir-list@redhat.com], [], 
> [http://libvirt.org])
> +AC_INIT([libvirt], [1.3.0], [libvir-list@redhat.com], [], 
> [http://libvirt.org])
>  AC_CONFIG_SRCDIR([src/libvirt.c])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> 

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

[libvirt] [PATCH] Post-release version bump to 1.3.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---

We've dropped support for old QEMU and there was a vote to change version to
1.3.0, let's make it official.

 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index f481c50..4b7c9ed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General 
Public
 dnl License along with this library.  If not, see
 dnl .
 
-AC_INIT([libvirt], [1.2.22], [libvir-list@redhat.com], [], 
[http://libvirt.org])
+AC_INIT([libvirt], [1.3.0], [libvir-list@redhat.com], [], [http://libvirt.org])
 AC_CONFIG_SRCDIR([src/libvirt.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
-- 
2.6.3

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

Re: [libvirt] [PATCH v2 7/7] qemu: Use qemuProcessLaunch in migration Prepare phase

[Pavel Hrdina]

On Thu, Nov 19, 2015 at 01:09:21PM +0100, Jiri Denemark wrote:
> Using qemuProcess{Init,Launch,FinishStartup} allows us to run
> pre-migration commands on destination before asking QEMU to wait for
> incoming migration data.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_migration.c | 68 
> ++-
>  1 file changed, 44 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index b6525df..fe9b1ff 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3291,14 +3291,16 @@ qemuMigrationPrepareCleanup(virQEMUDriverPtr driver,
>  qemuDomainObjDiscardAsyncJob(driver, vm);
>  }
>  
> -static char *
> +static qemuProcessIncomingDefPtr
>  qemuMigrationPrepareIncoming(virDomainObjPtr vm,
>   bool tunnel,
>   const char *protocol,
>   const char *listenAddress,
> - unsigned short port)
> + unsigned short port,
> + int fd)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> +qemuProcessIncomingDefPtr inc = NULL;
>  char *migrateFrom = NULL;
>  
>  if (tunnel) {
> @@ -3361,8 +3363,11 @@ qemuMigrationPrepareIncoming(virDomainObjPtr vm,
>  goto cleanup;
>  }
>  
> +inc = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, fd, NULL);
> +
>   cleanup:
> -return migrateFrom;
> +VIR_FREE(migrateFrom);
> +return inc;
>  }
>  
>  static int
> @@ -3393,8 +3398,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  char *xmlout = NULL;
>  unsigned int cookieFlags;
>  virCapsPtr caps = NULL;
> -char *migrateFrom = NULL;
> +qemuProcessIncomingDefPtr incoming = NULL;
>  bool taint_hook = false;
> +bool stopProcess = false;
> +bool relabel = false;
> +int rv;
>  
>  virNWFilterReadLockFilterUpdates();
>  
> @@ -3528,28 +3536,29 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  goto stopjob;
>  }
>  
> -virObjectUnref(priv->qemuCaps);
> -priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
> -vm->def->emulator,
> -vm->def->os.machine);
> -if (!priv->qemuCaps)
> +if ((rv = qemuProcessInit(driver, vm, true)) < 0) {
> +if (rv == -1)
> +stopProcess = true;
>  goto stopjob;
> +}
> +stopProcess = true;

With the changes in 1/7 and 5/7 this could be simple:

if (qemuProcessInit(driver, vm, true) < 0)
goto endjob/endmigjob/stobjob;

And every other goto will jump to 'stop' to make sure that the qemuProcess is
stopped too.

>  
> -if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol,
> - listenAddress, port)))
> +if (!(incoming = qemuMigrationPrepareIncoming(vm, tunnel, protocol,
> +  listenAddress, port,
> +  dataFD[0])))
>  goto stopjob;
> +dataFD[0] = -1; /* the FD is now owned by incoming */
>  
> -/* Start the QEMU daemon, with the same command-line arguments plus
> - * -incoming $migrateFrom
> - */
> -if (qemuProcessStart(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
> - migrateFrom, dataFD[0], NULL, NULL,
> - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START,
> - VIR_QEMU_PROCESS_START_PAUSED |
> - VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) {
> -virDomainAuditStart(vm, "migrated", false);
> +rv = qemuProcessLaunch(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
> +   incoming, NULL,
> +   VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START,
> +   VIR_QEMU_PROCESS_START_AUTODESTROY);
> +if (rv < 0) {
> +if (rv == -2)
> +relabel = true;
>  goto stopjob;
>  }
> +relabel = true;
>  
>  if (tunnel) {
>  if (virFDStreamOpen(st, dataFD[1]) < 0) {

[...]

Otherwise seems ok,

Pavel

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

Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for perf event

[Jiri Denemark]

On Tue, Nov 17, 2015 at 16:00:44 +0800, Qiaowei Ren wrote:
> Add remote support for perf event.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  daemon/remote.c  | 60 
> 
>  src/remote/remote_driver.c   | 49 
>  src/remote/remote_protocol.x | 32 ++-
>  src/remote_protocol-structs  | 20 +++
>  4 files changed, 160 insertions(+), 1 deletion(-)

This will need to be changed to match the new style of the API. But,
since you are adding a new API, would you mind creating a patch for
libvirt-python to add this new API to python bindings?

Jirka

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

Re: [libvirt] [PATCH v2 5/7] qemu: Kill QEMU process if Prepare phase fails

[Pavel Hrdina]

On Thu, Nov 19, 2015 at 01:09:19PM +0100, Jiri Denemark wrote:
> Some failure paths in qemuMigrationPrepareAny forgot to kill the just
> started QEMU process. This patch fixes this by combining 'stop' and
> 'endjob' label into a new label 'stopjob'. This name was chosen to avoid
> confusion with the most common semantics of 'endjob'. Normally, 'endjob'
> is always called at the end of an API to stop the job we entered at the
> beginning. In qemuMigrationPrepareAny we only want to stop the job in
> failure path; on success we need to carry the job over to the Finish
> phase.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> ACKed in version 1
> 
> Version 2:
> - no change
> 
>  src/qemu/qemu_migration.c | 31 ---
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 4ab6ab7..9a4f19f 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3522,7 +3522,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  (pipe(dataFD) < 0 || virSetCloseExec(dataFD[1]) < 0)) {
>  virReportSystemError(errno, "%s",
>   _("cannot create pipe for tunnelled 
> migration"));
> -goto endjob;
> +goto stopjob;
>  }
>  
>  virObjectUnref(priv->qemuCaps);
> @@ -3530,11 +3530,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  vm->def->emulator,
>  vm->def->os.machine);
>  if (!priv->qemuCaps)
> -goto endjob;
> +goto stopjob;
>  
>  if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol,
>   listenAddress, port)))
> -goto endjob;
> +goto stopjob;
>  
>  /* Start the QEMU daemon, with the same command-line arguments plus
>   * -incoming $migrateFrom
> @@ -3545,14 +3545,14 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>   VIR_QEMU_PROCESS_START_PAUSED |
>   VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) {
>  virDomainAuditStart(vm, "migrated", false);
> -goto endjob;
> +goto stopjob;
>  }
>  
>  if (tunnel) {
>  if (virFDStreamOpen(st, dataFD[1]) < 0) {
>  virReportSystemError(errno, "%s",
>   _("cannot pass pipe for tunnelled 
> migration"));
> -goto stop;
> +goto stopjob;
>  }
>  dataFD[1] = -1; /* 'st' owns the FD now & will close it */
>  }
> @@ -3560,17 +3560,17 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  if (qemuMigrationSetCompression(driver, vm,
>  flags & VIR_MIGRATE_COMPRESSED,
>  QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
> -goto stop;
> +goto stopjob;
>  
>  if (STREQ_NULLABLE(protocol, "rdma") &&
>  virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) 
> {
> -goto stop;
> +goto stopjob;
>  }
>  
>  if (qemuMigrationSetPinAll(driver, vm,
> flags & VIR_MIGRATE_RDMA_PIN_ALL,
> QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
> -goto stop;
> +goto stopjob;
>  
>  if (mig->lockState) {
>  VIR_DEBUG("Received lockstate %s", mig->lockState);
> @@ -3593,7 +3593,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
>  nmigrate_disks, migrate_disks) < 0) {
>  /* error already reported */
> -goto endjob;
> +goto stopjob;
>  }
>  cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
>  }
> @@ -3608,7 +3608,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  }
>  
>  if (qemuDomainCleanupAdd(vm, qemuMigrationPrepareCleanup) < 0)
> -goto endjob;
> +goto stopjob;
>  
>  if (!(flags & VIR_MIGRATE_OFFLINE)) {
>  virDomainAuditStart(vm, "migrated", true);
> @@ -3647,12 +3647,13 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>  virNWFilterUnlockFilterUpdates();
>  return ret;
>  
> - stop:
> -virDomainAuditStart(vm, "migrated", false);
> -qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
> -VIR_QEMU_PROCESS_STOP_MIGRATED);
> + stopjob:
> +if (vm->def->id != -1) {
> +virDomainAuditStart(vm, "migrated", false);
> +qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
> +VIR_QEMU_PROCESS_STOP_MIGRATED);
> +}
>  
> - endjob:
>  qemuMigrationJobFinish(driver, vm);
>  goto cleanup;

I wouldn't join those two labels together regarding the change in 1/7 patch.
Just change all goto after qemuProcessStart to jump to stop.  About the naming

Re: [libvirt] [PATCH v2 3/7] qemu: Introduce qemuProcessFinishStartup

[Pavel Hrdina]

On Thu, Nov 19, 2015 at 01:09:17PM +0100, Jiri Denemark wrote:
> Finishes starting a new domain launched by qemuProcessLaunch.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 2:
> - save status before running VIR_HOOK_QEMU_OP_STARTED hook
> - rename qemuProcessFinish as qemuProcessFinishStartup
> - remove unused cfg from qemuProcessStart
> 
>  src/qemu/qemu_process.c | 78 
> -
>  src/qemu/qemu_process.h |  6 
>  2 files changed, 57 insertions(+), 27 deletions(-)
> 

ACK,

Pavel

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

Re: [libvirt] [PATCH v4 08/10] Add iommu info for pci on mocked sysfs

[Shivaprasad bhat]

Thanks for the comments Michal.. I'll fix them all in my next version.

Regards,
Shivaprasad

On Mon, Nov 23, 2015 at 10:34 PM, Michal Privoznik  wrote:
> On 14.11.2015 09:38, Shivaprasad G Bhat wrote:
>> The iommu group info can be used to check if the devices are bound/unbound
>> from vfio at the group level granualrity. Add the info to mock as to help
>> add test cases later.
>>
>> Signed-off-by: Shivaprasad G Bhat 
>> ---
>>  tests/virpcimock.c |  180 
>> +++-
>>  1 file changed, 164 insertions(+), 16 deletions(-)
>>
>> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
>> index 0724a36..837976a 100644
>> --- a/tests/virpcimock.c
>> +++ b/tests/virpcimock.c
>> @@ -127,9 +127,15 @@ struct pciDevice {
>>  int vendor;
>>  int device;
>>  int class;
>> +int iommu;
>>  struct pciDriver *driver;   /* Driver attached. NULL if attached to no 
>> driver */
>>  };
>>
>> +struct pciIommuGroup {
>> +int iommu;
>> +size_t nDevicesBoundToVFIO;/* Indicates the devices in the 
>> group */
>> +};
>> +
>>  struct fdCallback {
>>  int fd;
>>  char *path;
>> @@ -141,6 +147,9 @@ size_t nPCIDevices = 0;
>>  struct pciDriver **pciDrivers = NULL;
>>  size_t nPCIDrivers = 0;
>>
>> +struct pciIommuGroup **pciIommuGroups = NULL;
>> +size_t npciIommuGroups = 0;
>> +
>>  struct fdCallback *callbacks = NULL;
>>  size_t nCallbacks = 0;
>>
>> @@ -325,6 +334,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
>>  char *configSrc;
>>  char tmp[32];
>>  struct stat sb;
>> +char *iommugrouppath, *deviommupath, *iommugroupdevs = NULL;
>>
>>  if (VIR_STRDUP_QUIET(id, data->id) < 0)
>>  ABORT_OOM();
>> @@ -387,6 +397,25 @@ pci_device_new_from_stub(const struct pciDevice *data)
>>  ABORT("@tmp overflow");
>>  make_file(devpath, "class", tmp, -1);
>>
>> +if (virAsprintfQuiet(&deviommupath, "%s/iommu_group", devpath) < 0 ||
>> +virAsprintfQuiet(&iommugrouppath, "%s/iommu_groups/%d",
>> + fakesysfsdir, dev->iommu) < 0)
>> +ABORT("@deviommupath overflow");
>
> Technically this is not overflow rather than OOM. I guess you've just copied 
> a pattern just above these lines (not to be seen in this patch though), where 
> we really are snprintf()-ing into 32 bytes wide string. Here we have all the 
> memory, so this should be ABORT_OOM();
>
>> +
>> +if (symlink(iommugrouppath, deviommupath) < 0)
>> +ABORT("Unable to link device to iommu group");
>> +
>> +VIR_FREE(deviommupath);
>> +if (virAsprintfQuiet(&iommugroupdevs, "%s/devices/%s",
>> + iommugrouppath, dev->id) < 0)
>> +ABORT("@iommugroupdevs overflow");
>
> Same here.
>
>> +
>> +if (symlink(devpath, iommugroupdevs) < 0)
>> +ABORT("Unable to link iommu group devices to current device");
>> +
>> +VIR_FREE(iommugrouppath);
>> +VIR_FREE(iommugroupdevs);
>> +
>>  if (pci_device_autobind(dev) < 0)
>>  ABORT("Unable to bind: %s", data->id);
>>
>> @@ -435,7 +464,95 @@ pci_device_autobind(struct pciDevice *dev)
>>  return pci_driver_bind(driver, dev);
>>  }
>>
>> +static void
>> +pci_iommu_new(int num)
>> +{
>> +char *iommupath;
>> +struct pciIommuGroup *iommuGroup;
>> +
>> +if (VIR_ALLOC_QUIET(iommuGroup) < 0)
>> +ABORT_OOM();
>> +
>> +iommuGroup->iommu = num;
>> +iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to vfio by 
>> default */
>> +
>> +if (virAsprintfQuiet(&iommupath, "%s/iommu_groups/%d/devices", 
>> fakesysfsdir, num) < 0)
>> +ABORT_OOM();
>> +
>> +if (virFileMakePath(iommupath) < 0)
>> +ABORT("Unable to create: %s", iommupath);
>> +
>> +if (VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups, 
>> iommuGroup) < 0)
>> +ABORT_OOM();
>
> @iommupath is no longer needed and leaked.
>
>> +}
>> +
>> +static int
>> +pci_vfio_release_iommu(struct pciDevice *device)
>> +{
>> +char *vfiopath = NULL;
>> +int ret = -1;
>> +size_t i = 0;
>> +
>> +for (i = 0; i < npciIommuGroups; i++) {
>> +if (pciIommuGroups[i]->iommu == device->iommu)
>> +break;
>> +}
>> +
>> +if (i != npciIommuGroups) {
>> +if (pciIommuGroups[i]->nDevicesBoundToVFIO == 0) {
>> +ret = 0;
>> +goto cleanup;
>> +}
>
> This is somewhat confusing to me. This can happen due to a bug in our code - 
> in that case I'd expect an error to be thrown.
>
>> +pciIommuGroups[i]->nDevicesBoundToVFIO--;
>> +if (!pciIommuGroups[i]->nDevicesBoundToVFIO) {
>> +if (virAsprintfQuiet(&vfiopath, "%s/dev/vfio/%d",
>> + fakesysfsdir, device->iommu) < 0) {
>> +errno = ENOMEM;
>> +goto cleanup;
>> +}
>> +if (unlink(vfiopath) < 0)
>> +goto cleanup;
>> +}
>> +}
>> +
>> +ret = 0;
>> + cleanup:
>> +V