[libvirt] [RFC PATCH 0/2] nodeinfo: PPC64: Fix topology and siblings info on capabilities and nodeinfo

[Shivaprasad G Bhat]

The nodeinfo output was fixed earlier to reflect the actual cpus available in
KVM mode on PPC64. The earlier fixes covered the aspect of not making a host
look overcommitted when its not. The current fixes are aimed at helping the
users make better decisions on the kind of guest cpu topology that can be
supported on the given sucore_per_core setting of KVM host and also hint the
way to pin the guest vcpus efficiently.

I am planning to add some test cases once the approach is accepted.

With respect to Patch 2:
The second patch adds a new element to the cpus tag and I need your inputs on
if that is okay. Also if there is a better way. I am not sure if the existing
clients have RNG checks that might fail with the approach. Or if the checks
are not enoforced on the elements but only on the tags.

With my approach if the rng checks pass, the new element "capacity" even if
ignored by many clients would have no impact except for PPC64.

To the extent I looked at code, the siblings changes dont affect existing
libvirt functionality. Please do let me know otherwise.


---

Shivaprasad G Bhat (2):
  nodeinfo: Reflect guest usable host topology on PPC64
  Introduce capacity to virCapsHostNUMACellCPU to help vcpu pinning 
decisions


 src/conf/capabilities.c|3 +
 src/conf/capabilities.h|1 
 src/nodeinfo.c |   51 ++--
 tests/vircaps2xmldata/vircaps-basic-4-4-2G.xml |   32 ---
 tests/vircaps2xmltest.c|1 
 5 files changed, 67 insertions(+), 21 deletions(-)

--
Signature

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

Re: [libvirt] [PATCH 3/4] vz: fix notification subscription

[Nikolay Shirokovskiy]

On 28.01.2016 18:38, Mikhail Feoktistov wrote:
> Bug cause:
> Update the domain that is subscribed to hypervisor notification.
> LoadDomain() rewrites notifications fields in vzDomObj structure and makes 
> domain as "unsubscribed".
> Fix:
> Initialize notification fields in vzDomObj only if we create a new domain.
> And do not reinitialize these fields if we update domain (by calling 
> LoadDomain with olddom argument)
> ---
>  src/vz/vz_sdk.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 40ab2d9..6fb2a97 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -1270,6 +1270,12 @@ prlsdkLoadDomain(vzConnPtr privconn,
>  if (!olddom) {
>  if (VIR_ALLOC(pdom) < 0)
>  goto error;
> +pdom->cache.stats = PRL_INVALID_HANDLE;
> +pdom->cache.count = -1;
> +if (virCondInit(>cache.cond) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot 
> initialize condition"));
> +goto error;
> +}
>  } else {
>  pdom = olddom->privateData;
>  }
> @@ -1281,13 +1287,6 @@ prlsdkLoadDomain(vzConnPtr privconn,
>  
>  def->id = -1;
>  
> -pdom->cache.stats = PRL_INVALID_HANDLE;
> -pdom->cache.count = -1;
> -if (virCondInit(>cache.cond) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize 
> condition"));
> -goto error;
> -}
> -
>  if (prlsdkGetDomainIds(sdkdom, >name, def->uuid) < 0)
>  goto error;
>  
> 
ACK

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

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

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:34PM +0800, Qiaowei Ren wrote:
> This patch implement the internal driver API for perf event into
> qemu driver.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  include/libvirt/libvirt-domain.h |   1 +
>  src/qemu/qemu_domain.h   |   3 +
>  src/qemu/qemu_driver.c   | 133 
> +++
>  src/qemu/qemu_process.c  |   6 ++
>  4 files changed, 143 insertions(+)
> 


> @@ -19427,6 +19508,55 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>  
>  #undef QEMU_ADD_COUNT_PARAM
>  
> +static int
> +qemuDomainGetStatsPerfCmt(virPerfPtr perf,
> +  virDomainStatsRecordPtr record,
> +  int *maxparams)
> +{
> +uint64_t cache = 0;
> +
> +if (virPerfReadEvent(perf, VIR_PERF_EVENT_CMT, ) < 0)
> +return -1;
> +
> +if (virTypedParamsAddULLong(>params,
> +>nparams,
> +maxparams,
> +"perf.cache",
> +cache) < 0)
> +return -1;
> +
> +return 0;
> +}
> +
> +static int
> +qemuDomainGetStatsPerf(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> +   virDomainObjPtr dom,
> +   virDomainStatsRecordPtr record,
> +   int *maxparams,
> +   unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +size_t i;
> +qemuDomainObjPrivatePtr priv = dom->privateData;
> +int ret = -1;
> +
> +for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
> +if (!virPerfEventIsEnabled(priv->perf, i))
> + continue;
> +
> +switch (i) {
> +case VIR_PERF_EVENT_CMT:
> +if (qemuDomainGetStatsPerfCmt(priv->perf, record, maxparams))
> +goto cleanup;

I guess that should be checking  '< 0'

> +break;
> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +return ret;
> +}
> +
>  typedef int
>  (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver,
>virDomainObjPtr dom,
> @@ -19447,6 +19577,7 @@ static struct qemuDomainGetStatsWorker 
> qemuDomainGetStatsWorkers[] = {
>  { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
>  { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
>  { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true },
> +{ qemuDomainGetStatsPerf, VIR_DOMAIN_STATS_PERF, false },
>  { NULL, 0, false }
>  };
>  
> @@ -20207,6 +20338,8 @@ static virHypervisorDriver qemuHypervisorDriver = {
>  .domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */
>  .domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */
>  .domainSendKey = qemuDomainSendKey, /* 0.9.4 */
> +.domainGetPerfEvents = qemuDomainGetPerfEvents, /* 1.3.1 */
> +.domainSetPerfEvents = qemuDomainSetPerfEvents, /* 1.3.1 */
>  .domainBlockJobAbort = qemuDomainBlockJobAbort, /* 0.9.4 */
>  .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */
>  .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4201962..8679d29 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4844,6 +4844,10 @@ qemuProcessLaunch(virConnectPtr conn,
>  if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0)
>  goto cleanup;
>  
> +priv->perf = virPerfNew();
> +if (!priv->perf)
> +goto cleanup;
> +

You also need todo this in  qemuProcessReconnect so that it works
after libvirtd is restarted, and in qemuProcessAttach too


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] [libvirt-glib 2/3] Add GVirConfigDomainHostdevPci

[Zeeshan Ali (Khattak)]

Add API to read and write PCI hostdev nodes.
---
 libvirt-gconfig/Makefile.am|   2 +
 .../libvirt-gconfig-domain-hostdev-pci.c   | 211 +
 .../libvirt-gconfig-domain-hostdev-pci.h   |  80 
 libvirt-gconfig/libvirt-gconfig-domain-hostdev.c   |   2 +-
 libvirt-gconfig/libvirt-gconfig.h  |   1 +
 libvirt-gconfig/libvirt-gconfig.sym|   9 +
 6 files changed, 304 insertions(+), 1 deletion(-)
 create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c
 create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.h

diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index 4294bab..245313d 100644
--- a/libvirt-gconfig/Makefile.am
+++ b/libvirt-gconfig/Makefile.am
@@ -51,6 +51,7 @@ GCONFIG_HEADER_FILES = \
libvirt-gconfig-domain-graphics-spice.h \
libvirt-gconfig-domain-graphics-vnc.h \
libvirt-gconfig-domain-hostdev.h \
+   libvirt-gconfig-domain-hostdev-pci.h \
libvirt-gconfig-domain-input.h \
libvirt-gconfig-domain-interface.h \
libvirt-gconfig-domain-interface-bridge.h \
@@ -143,6 +144,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-domain-graphics-spice.c \
libvirt-gconfig-domain-graphics-vnc.c \
libvirt-gconfig-domain-hostdev.c \
+   libvirt-gconfig-domain-hostdev-pci.c \
libvirt-gconfig-domain-input.c \
libvirt-gconfig-domain-interface.c \
libvirt-gconfig-domain-interface-bridge.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c 
b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c
new file mode 100644
index 000..ed1d146
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev-pci.c
@@ -0,0 +1,211 @@
+/*
+ * libvirt-gconfig-domain-hostdev.c: libvirt domain hostdev configuration
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * .
+ *
+ * Authors: Zeeshan Ali (Khattak) 
+ *  Christophe Fergeau 
+ */
+
+#include 
+
+#include "libvirt-gconfig/libvirt-gconfig.h"
+#include "libvirt-gconfig/libvirt-gconfig-private.h"
+
+#define GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_PRIVATE(obj)
 \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), 
GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI, GVirConfigDomainHostdevPciPrivate))
+
+struct _GVirConfigDomainHostdevPciPrivate
+{
+gboolean unused;
+};
+
+G_DEFINE_TYPE(GVirConfigDomainHostdevPci, gvir_config_domain_hostdev_pci, 
GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV);
+
+static void 
gvir_config_domain_hostdev_pci_class_init(GVirConfigDomainHostdevPciClass 
*klass)
+{
+g_type_class_add_private(klass, sizeof(GVirConfigDomainHostdevPciPrivate));
+}
+
+
+static void gvir_config_domain_hostdev_pci_init(GVirConfigDomainHostdevPci 
*hostdev)
+{
+hostdev->priv = GVIR_CONFIG_DOMAIN_HOSTDEV_PCI_GET_PRIVATE(hostdev);
+}
+
+/**
+ * gvir_config_domain_hostdev_pci_new:
+ *
+ * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1.
+ *
+ * Returns: a new #GVirConfigDomainHostdevPci
+ */
+GVirConfigDomainHostdevPci *gvir_config_domain_hostdev_pci_new(void)
+{
+GVirConfigObject *object;
+
+object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV_PCI,
+"hostdev", NULL);
+gvir_config_object_set_attribute(object, "mode", "subsystem", NULL);
+gvir_config_object_set_attribute(object, "type", "pci", NULL);
+
+return GVIR_CONFIG_DOMAIN_HOSTDEV_PCI(object);
+}
+
+/**
+ * gvir_config_domain_hostdev_pci_new_from_xml:
+ * @xml: xml data to create the host device from
+ * @error: return location for a #GError, or NULL
+ *
+ * Creates a new #GVirConfigDomainHostdevPci with a reference count of 1.
+ * The host device object will be created using the XML description stored
+ * in @xml. This is a fragment of libvirt domain XML whose root node is
+ * hostdev.
+ *
+ * Returns: a new #GVirConfigDomainHostdevPci, or NULL if @xml failed to
+ * be parsed.
+ */
+GVirConfigDomainHostdevPci 

[libvirt] [libvirt-glib 3/3] Add test for GVirConfigDomainHostdevPci API

[Zeeshan Ali (Khattak)]

---
 tests/test-gconfig.c| 30 +
 tests/xml/gconfig-domain-device-pci-hostdev.xml | 11 +
 2 files changed, 41 insertions(+)
 create mode 100644 tests/xml/gconfig-domain-device-pci-hostdev.xml

diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c
index be55ef9..c3711f1 100644
--- a/tests/test-gconfig.c
+++ b/tests/test-gconfig.c
@@ -709,6 +709,34 @@ static void test_domain_device_usb_redir(void)
 g_object_unref(G_OBJECT(domain));
 }
 
+static void test_domain_device_pci_hostdev(void)
+{
+GVirConfigDomain *domain;
+GVirConfigDomainAddressPci *address;
+GVirConfigDomainHostdevPci *hostdev;
+
+domain = gvir_config_domain_new();
+
+hostdev = gvir_config_domain_hostdev_pci_new();
+
gvir_config_domain_hostdev_set_boot_order(GVIR_CONFIG_DOMAIN_HOSTDEV(hostdev), 
1);
+gvir_config_domain_hostdev_pci_set_managed(hostdev, TRUE);
+gvir_config_domain_hostdev_pci_set_rom(hostdev, "/etc/fake/boot.bin", 
TRUE);
+
+address = gvir_config_domain_address_pci_new();
+gvir_config_domain_address_pci_set_domain(address, 1);
+gvir_config_domain_address_pci_set_bus(address, 2);
+gvir_config_domain_address_pci_set_slot(address, 3);
+gvir_config_domain_address_pci_set_function(address, 4);
+gvir_config_domain_hostdev_pci_set_address(hostdev, address);
+g_object_unref(G_OBJECT(address));
+
+gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE (hostdev));
+g_object_unref(G_OBJECT(hostdev));
+
+check_xml(domain, "gconfig-domain-device-pci-hostdev.xml");
+
+g_object_unref(G_OBJECT(domain));
+}
 
 int main(int argc, char **argv)
 {
@@ -739,6 +767,8 @@ int main(int argc, char **argv)
 test_domain_device_channel);
 g_test_add_func("/libvirt-gconfig/domain-device-usb-redir",
 test_domain_device_usb_redir);
+g_test_add_func("/libvirt-gconfig/domain-device-pci-hostdev",
+test_domain_device_pci_hostdev);
 
 return g_test_run();
 }
diff --git a/tests/xml/gconfig-domain-device-pci-hostdev.xml 
b/tests/xml/gconfig-domain-device-pci-hostdev.xml
new file mode 100644
index 000..70e32ac
--- /dev/null
+++ b/tests/xml/gconfig-domain-device-pci-hostdev.xml
@@ -0,0 +1,11 @@
+
+  
+
+  
+  
+  
+
+  
+
+  
+
-- 
2.5.0

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

[libvirt] [libvirt-glib 1/3] Add GVirConfigDomainHostdev

[Zeeshan Ali (Khattak)]

Add API to read and write domain/devices/hostdev nodes. This patch only
adds the baseclass and hence is not useful on it's own. A more specific
subclass to represent PCI devices will be added in a following patch.
---
 libvirt-gconfig/Makefile.am|   2 +
 .../libvirt-gconfig-domain-device-private.h|   3 +
 libvirt-gconfig/libvirt-gconfig-domain-device.c|   2 +-
 libvirt-gconfig/libvirt-gconfig-domain-hostdev.c   | 180 +
 libvirt-gconfig/libvirt-gconfig-domain-hostdev.h   |  76 +
 libvirt-gconfig/libvirt-gconfig.h  |   1 +
 libvirt-gconfig/libvirt-gconfig.sym|  11 ++
 7 files changed, 274 insertions(+), 1 deletion(-)
 create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev.c
 create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-hostdev.h

diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index 77b2032..4294bab 100644
--- a/libvirt-gconfig/Makefile.am
+++ b/libvirt-gconfig/Makefile.am
@@ -50,6 +50,7 @@ GCONFIG_HEADER_FILES = \
libvirt-gconfig-domain-graphics-sdl.h \
libvirt-gconfig-domain-graphics-spice.h \
libvirt-gconfig-domain-graphics-vnc.h \
+   libvirt-gconfig-domain-hostdev.h \
libvirt-gconfig-domain-input.h \
libvirt-gconfig-domain-interface.h \
libvirt-gconfig-domain-interface-bridge.h \
@@ -141,6 +142,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-domain-graphics-sdl.c \
libvirt-gconfig-domain-graphics-spice.c \
libvirt-gconfig-domain-graphics-vnc.c \
+   libvirt-gconfig-domain-hostdev.c \
libvirt-gconfig-domain-input.c \
libvirt-gconfig-domain-interface.c \
libvirt-gconfig-domain-interface-bridge.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h 
b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
index 062c0e2..c45e1df 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-device-private.h
@@ -43,6 +43,9 @@ GVirConfigDomainDevice *
 gvir_config_domain_graphics_new_from_tree(GVirConfigXmlDoc *doc,
   xmlNodePtr tree);
 GVirConfigDomainDevice *
+gvir_config_domain_hostdev_new_from_tree(GVirConfigXmlDoc *doc,
+ xmlNodePtr tree);
+GVirConfigDomainDevice *
 gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc,
xmlNodePtr tree);
 GVirConfigDomainDevice *
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device.c 
b/libvirt-gconfig/libvirt-gconfig-domain-device.c
index 3d2b9b3..8a75cea 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-device.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-device.c
@@ -66,7 +66,7 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc *doc,
 } else if (xmlStrEqual(tree->name, (xmlChar*)"lease")) {
 goto unimplemented;
 } else if (xmlStrEqual(tree->name, (xmlChar*)"hostdev")) {
-goto unimplemented;
+return gvir_config_domain_hostdev_new_from_tree(doc, tree);
 } else if (xmlStrEqual(tree->name, (xmlChar*)"redirdev")) {
 type = GVIR_CONFIG_TYPE_DOMAIN_REDIRDEV;
 } else if (xmlStrEqual(tree->name, (xmlChar*)"smartcard")) {
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c 
b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c
new file mode 100644
index 000..42eb184
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-hostdev.c
@@ -0,0 +1,180 @@
+/*
+ * libvirt-gconfig-domain-hostdev.c: libvirt domain hostdev configuration
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * .
+ *
+ * Authors: Zeeshan Ali (Khattak) 
+ *  Christophe Fergeau 
+ */
+
+#include 
+
+#include "libvirt-gconfig/libvirt-gconfig.h"
+#include "libvirt-gconfig/libvirt-gconfig-private.h"
+
+#define GVIR_CONFIG_DOMAIN_HOSTDEV_GET_PRIVATE(obj) \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_HOSTDEV, 

Re: [libvirt] [PATCH 4/4] logical: Adjust regex for devices

[John Ferlan]

On 01/28/2016 06:30 AM, Pavel Hrdina wrote:
> On Wed, Jan 27, 2016 at 03:26:12PM -0500, John Ferlan wrote:
>> ...

 However, after some more "investigation" of the environment - I think
 perhaps there's still a couple of loose ends. Keeping the 'segtype'
 field may be necessary/useful... Details follow if interested ;-)
>>>
>>> Yes, you're right and I did a bad job during review.  The segtype is 
>>> required
>>> and we should always consider it, there is like 20 segtypes right now and 
>>> some
>>> of them could also use 'stripes'.
>>>
>>
>> The 'segtype' is currently only required because commit id '82c1740a'
>> added 'segtype' and 'stripes' to resolve a bz (or more than one
>> depending on how far you chase) by using 'segtype' to determine whether
>> more than one existed. It also did quite a few things in one step which
>> by today's review standards is a bad thing ;-).
>>
>> However, that only worked for "striped" lv's. If there was a "mirror"
>> lv, then while the mirror could be found, the vol-dumpxml output is
>> wrong because it only parsed 1 extent (incorrectly at that):
> 
> That leads me to conclusion, that we should parse only the segtypes that we 
> know
> how to parse.  The rest should be ignored.
> 
>>
>>   
>> 
>>   
>> 
>>   
>>
>> instead of something like (if using 'stripes' to get nsegments):
>>
>>   
>> 
>>   
>> 
>> 
>>   
>> 
>>   
>>
>> Linking 'nsegments' to 'striped' lv's is a "limitation".
>>
>> Anyway, dropping 'segtype' wasn't necessarily bad unless you needed
>> "more information", such as perhaps the lv thin pool source when
>> nsegments == 0. This becomes obvious once the change to use "(\\S*)"
>> instead of "(\\S+)" hits. Now we can "see" thin lv's in a volume group.
>> Not sure what else becomes visible, though.  The problem is you then get:
> 
> But we need more information and we propagate those information to user via 
> our
> API and that's why I think that we should parse only those volume which we 
> know
> how to parse to pass correct values to user.

I understand your point, but what is that list? What happens if/when we
"choose" something that works today?

The issues with the code now:

1. It doesn't recognize a thin lv

2. It doesn't parse a mirror or raid segtype extent correctly

Both are based around using the 'segtype' to make a decision to use the
'stripes' field in order to get the number of segments found. While it
does accurately give the number of devices for striped, mirror, raid,
thin, thin-pool, and linear lv's - it's not documented that way.

So by using 'segtype' to limit we've already caused an issue, so I'm not
sure using that to limit what we parse.

Using it to make a decision about how to parse a specific segtype I
think would be OK.

Secondarily, using 'stripes' as the count of extents is probably wrong
too. The count of segments should probably be made by actively parsing
the devices field and doing the VIR_APPEND_ELEMENT as you originally
suggested each time we parse something that has "string(#)", where
string and (#) are the parsed elements.

Let's see where this takes me ;-)

John
> 
> Pavel
> 
>>
>>  
>>  
>>
>> But that's fixable... The interesting part about  is that it's
>> an output only (virStorageVolDefParseXML doesn't read it). So, by adding
>> a new parse field 'pool_lv', we can then check 'segtype' to be "thin"
>> and if so, store the new field in a new vol->source.thin_pool field.
>>
>> Still cannot create a thin lv pool/volume without using the lvcreate
>> command. Nor can one create a mirror or stripe lv using libvirt's
>> vol-create command. One just cannot "find" a thin lv right now...
>>
>> John
>>

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

[libvirt] [PATCH 1/4] vz: make output arguments in prlsdkGetDomainIds as optional

[Mikhail Feoktistov]

prlsdkGetDomainIds() returns name and uuid for specified instance.
Now output arguments can be NULL.
It allows to get only necessary info(name or uuid).
---
 src/vz/vz_sdk.c | 34 +++---
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 7973d6a..8181149 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -346,25 +346,29 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
 PRL_UINT32 len;
 PRL_RESULT pret;
 
-len = 0;
-/* get name length */
-pret = PrlVmCfg_GetName(sdkdom, NULL, );
-prlsdkCheckRetGoto(pret, error);
+if (name) {
+len = 0;
+/* get name length */
+pret = PrlVmCfg_GetName(sdkdom, NULL, );
+prlsdkCheckRetGoto(pret, error);
 
-if (VIR_ALLOC_N(*name, len) < 0)
-goto error;
+if (VIR_ALLOC_N(*name, len) < 0)
+goto error;
 
-PrlVmCfg_GetName(sdkdom, *name, );
-prlsdkCheckRetGoto(pret, error);
+PrlVmCfg_GetName(sdkdom, *name, );
+prlsdkCheckRetGoto(pret, error);
+}
 
-len = sizeof(uuidstr);
-PrlVmCfg_GetUuid(sdkdom, uuidstr, );
-prlsdkCheckRetGoto(pret, error);
+if (uuid) {
+len = sizeof(uuidstr);
+PrlVmCfg_GetUuid(sdkdom, uuidstr, );
+prlsdkCheckRetGoto(pret, error);
 
-if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Domain UUID is malformed or empty"));
-goto error;
+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Domain UUID is malformed or empty"));
+goto error;
+}
 }
 
 return 0;
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] vz: remove unused struct field

[Mikhail Feoktistov]

ignore this patch
I include it into patch series.

26.01.2016 15:16, Mikhail Feoktistov пишет:

In commit 7039bb3c we have removed code that saves uuid to vzDomObj.uuid
So this field is no longer needed.
---
  src/vz/vz_sdk.c   | 5 -
  src/vz/vz_utils.h | 1 -
  2 files changed, 6 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index d610979..5e4c4ac 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -413,7 +413,6 @@ prlsdkDomObjFreePrivate(void *p)
  PrlHandle_Free(pdom->sdkdom);
  PrlHandle_Free(pdom->cache.stats);
  virCondDestroy(>cache.cond);
-VIR_FREE(pdom->uuid);
  VIR_FREE(pdom->home);
  VIR_FREE(p);
  };
@@ -1281,10 +1280,6 @@ prlsdkLoadDomain(vzConnPtr privconn,
  
  def->id = -1;
  
-/* we will remove this field in the near future, so let's set it

- * to NULL temporarily */
-pdom->uuid = NULL;
-
  pdom->cache.stats = PRL_INVALID_HANDLE;
  pdom->cache.count = -1;
  if (virCondInit(>cache.cond) < 0) {
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index b7a4c81..417f821 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -76,7 +76,6 @@ typedef struct _vzCountersCache vzCountersCache;
  
  struct vzDomObj {

  int id;
-char *uuid;
  char *home;
  PRL_HANDLE sdkdom;
  vzCountersCache cache;


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

Re: [libvirt] [PATCH 6/8] perf: reenable perf events when libvirtd restart

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:36PM +0800, Qiaowei Ren wrote:
> When libvirtd daemon restart, this patch will reenable those perf
> events previously enabled.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  src/qemu/qemu_driver.c | 36 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7d95364..09607ca 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -164,6 +164,9 @@ static int qemuDomainGetMaxVcpus(virDomainPtr dom);
>  static int qemuDomainManagedSaveLoad(virDomainObjPtr vm,
>   void *opaque);
>  
> +static int qemuDomainPerfRestart(virDomainObjPtr vm,
> + void *data);
> +
>  static int qemuOpenFile(virQEMUDriverPtr driver,
>  virDomainObjPtr vm,
>  const char *path, int oflags,
> @@ -939,6 +942,10 @@ qemuStateInitialize(bool privileged,
>  qemuDomainManagedSaveLoad,
>  qemu_driver);
>  
> +virDomainObjListForEach(qemu_driver->domains,
> +qemuDomainPerfRestart,
> +NULL);
> +
>  qemuProcessReconnectAll(conn, qemu_driver);
>  
>  qemu_driver->workerPool = virThreadPoolNew(0, 1, 0, 
> qemuProcessEventHandler, qemu_driver);
> @@ -3460,6 +3467,35 @@ qemuDomainManagedSaveLoad(virDomainObjPtr vm,
>  return ret;
>  }
>  
> +static int
> +qemuDomainPerfRestart(virDomainObjPtr vm,
> +  void *data ATTRIBUTE_UNUSED)
> +{
> +size_t i;
> +virDomainDefPtr def = vm->def;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +virPerfFree(priv->perf);
> +
> +priv->perf = virPerfNew();
> +if (!priv->perf)
> +return -1;
> +
> +for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
> +if (def->perf->events[i] &&
> +def->perf->events[i] == VIR_TRISTATE_BOOL_YES) {
> +if (virPerfEventEnable(priv->perf, i, vm->pid))
> +goto cleanup;
> +}
> +}
> +
> +return 0;
> +
> + cleanup:
> +virPerfFree(priv->perf);
> +return -1;
> +}
> +

Oh, I suggested we add this in qemuProcessReconnect in a previous
patch

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 10/34] qemu: Don't use priv->ncpus to iterate cgroup setting

[John Ferlan]

On 01/28/2016 08:57 AM, Peter Krempa wrote:
> On Sat, Jan 16, 2016 at 10:22:13 -0500, John Ferlan wrote:
>>
>>
>> On 01/14/2016 11:26 AM, Peter Krempa wrote:
>>> Iterate over all cpus skipping inactive ones.
>>> ---
>>>  src/qemu/qemu_driver.c | 15 ---
>>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>
>> Patch 7 introduces virDomainDefGetVcpumap (or GetOnlineVcpuMap) - why
>> not use that instead and then iterate the set bits.  This works, but the
>> less places that check for vcpu->online perhaps the better. Perhaps also
>> a way to reduce the decision points of using ->maxvcpus or the
>> virDomainDefGetVcpusMax call...
> 
> That would end up in two cases:
> 
> 1) The bitmap would be recalculated before every use.
> This increases complexity twofold since the function iterates the list
> once to assemble the bitmap and then you iterate the bitmap to get the
> objects.
> 
> 2) The bitmap would need to be stored persistently
> This again introduces two different places where data has to be stored.
> It either then will require us to keep it in sync the two places or
> remove one and the def->vcpus and def->vcpumap would need to be used in
> parallel always.
> 
> I don't like either of those since the target was to remove two places
> storing data about one object.
> 
> Peter
> 

For as many places that iterate and filter based on ->online - just
figured perhaps it may work to generate and store that bitmap. When all
is said and done - will there be more places iterating defs->vcpus
without caring about online or more places that mainly care about
online?  I searched on virDomainDefGetVcpu callers after the end of this
series and found 21 callers.  Of those 21, I counted 17 that make the
check immediately for online. Since the index for online bitmap would be
the same as vcpus[], it just seems logical to me to utilize it. I assume
eventually vcpupids is going away and vcpus will store the pid (or -1)
for the 'online' vcpu.

As I see the code now 'virDomainDefGetVcpumap' is called from just one
place (qemuDomainGetCPUStats - patch 11) for the express purpose of
generating a bitmap of online vCPU's in order to iterate the bitmap to
determine which guestvcpus to get PercpuStats about (or more
specifically so that virCgroupGetPercpuVcpuSum only uses vcpupids array
indices that should match our online map). Conversely there is other
code that uses the virDomainDefGetVcpusMax in order to get both the
index into vcpus and vcpupids (which today are in the same order). So
the code is now getting the same data two different ways because of
course vcpupids exists.

I probably should have also noted in patch 11 review that whole fetch is
unnecessary if (start_cpu == -1), but was thinking at the time more in
terms of having the map available/stored. It's also unnecessary to
generate the bitmap if params == 0 or ncpus == 0 since it won't be used.


Again - it was a suggestion not a requirement. Just trying to provide a
perspective from someone not in the middle of making the changes...

John

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

Re: [libvirt] [PATCH] pci: Use bool return type for some virPCIDeviceGet*() functions

[John Ferlan]

On 01/28/2016 03:21 AM, Andrea Bolognani wrote:
> The affected functions are:
> 
>   virPCIDeviceGetManaged()
>   virPCIDeviceGetUnbindFromStub()
>   virPCIDeviceGetRemoveSlot()
>   virPCIDeviceGetReprobe()
> 
> Change their return type from unsigned int to bool: the corresponding
> members in struct _virPCIDevice are defined as bool, and even the
> corresponding virPCIDeviceSet*() functions take a bool value as input
> so there's no point in these functions having unsigned int as return
> type.
> 
> Suggested-by: John Ferlan 
> ---
>  src/util/virpci.c | 8 
>  src/util/virpci.h | 8 
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 

ACK

John

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

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

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:32PM +0800, Qiaowei Ren wrote:
> Add remote support for perf event.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  daemon/remote.c  | 47 
> 
>  src/remote/remote_driver.c   | 39 
>  src/remote/remote_protocol.x | 30 +++-
>  src/remote_protocol-structs  | 18 +
>  4 files changed, 133 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

Re: [libvirt] [PATCH 7/8] virsh: implement new command to support perf

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:37PM +0800, Qiaowei Ren wrote:
> This patch add new perf command to enable/disable perf event
> for a guest domain.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  tools/virsh-domain.c | 128 
> +++
>  tools/virsh.pod  |  20 
>  2 files changed, 148 insertions(+)

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 8/8] virsh: extend domstats command

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:38PM +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(-)

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 2/4] vz: remove unused struct field

[Nikolay Shirokovskiy]

On 28.01.2016 18:38, Mikhail Feoktistov wrote:
> In commit 7039bb3c we have removed code that saves uuid to vzDomObj.uuid
> So this field is no longer needed.
> ---
>  src/vz/vz_sdk.c   | 5 -
>  src/vz/vz_utils.h | 1 -
>  2 files changed, 6 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 8181149..40ab2d9 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -417,7 +417,6 @@ prlsdkDomObjFreePrivate(void *p)
>  PrlHandle_Free(pdom->sdkdom);
>  PrlHandle_Free(pdom->cache.stats);
>  virCondDestroy(>cache.cond);
> -VIR_FREE(pdom->uuid);
>  VIR_FREE(pdom->home);
>  VIR_FREE(p);
>  };
> @@ -1282,10 +1281,6 @@ prlsdkLoadDomain(vzConnPtr privconn,
>  
>  def->id = -1;
>  
> -/* we will remove this field in the near future, so let's set it
> - * to NULL temporarily */
> -pdom->uuid = NULL;
> -
>  pdom->cache.stats = PRL_INVALID_HANDLE;
>  pdom->cache.count = -1;
>  if (virCondInit(>cache.cond) < 0) {
> diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
> index 84cf08f..0fbbfc7 100644
> --- a/src/vz/vz_utils.h
> +++ b/src/vz/vz_utils.h
> @@ -75,7 +75,6 @@ typedef struct _vzCountersCache vzCountersCache;
>  
>  struct vzDomObj {
>  int id;
> -char *uuid;
>  char *home;
>  PRL_HANDLE sdkdom;
>  vzCountersCache cache;
> 

ACK

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

[libvirt] [RFC PATCH 2/2] Introduce capacity to virCapsHostNUMACellCPU to help vcpu pinning decisions

[Shivaprasad G Bhat]

The cpus tag in NUMA cell today explains core's id, socket id and the
siblings list. The sibling list is used by users to pin the vcpus of a guest
from the same guest-core to threads from the same host-core. The host
topology gives a hint of how many max siblings one might expect in the
siblings list.

For PPC64, the previous assumptions fail to make sense when the subcores are
in use. The core is split into subcores and the siblings that libvirt sees from
the sysfs are list of subcores instead of siblings. The real siblings of
subcore are in fact offline on host and brought online by the KVM scheduler
in guest context.

So, the best way to achieve efficiency in this case is to pin all the guest
threads from same guest-core to the same subcore(the primary thread which is
online). The new parameter "capacity" reflects the thread capacity of the
core/subcore and how many vcpus from the same guest-core be pinned to the
primary thread.

On PPC64, the capacity will be set to the threads_per_subcore and the
siblings list will contain "self" cpu. The capacity thus can be used to
indicate how many vcpus from guest-core can be pinned to the same subcore.

On other archs, the capacity will be set to "1" indicating to treat each core
in the sibling list to be pinned only once.

Signed-off-by: Shivaprasad G Bhat 
---
 src/conf/capabilities.c|3 +-
 src/conf/capabilities.h|1 +
 src/nodeinfo.c |   34 +---
 tests/vircaps2xmldata/vircaps-basic-4-4-2G.xml |   32 +++
 tests/vircaps2xmltest.c|1 +
 5 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 86ea212..62ade2e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -799,7 +799,8 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf,
 virBufferAsprintf(buf, "\n", cells[i]->ncpus);
 virBufferAdjustIndent(buf, 2);
 for (j = 0; j < cells[i]->ncpus; j++) {
-virBufferAsprintf(buf, "cpus[j].id);
+virBufferAsprintf(buf, "cpus[j].id, 
cells[i]->cpus[j].capacity);
 
 if (cells[i]->cpus[j].siblings) {
 if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings)))
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 1754b13..94aa729 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -90,6 +90,7 @@ typedef struct _virCapsHostNUMACellCPU virCapsHostNUMACellCPU;
 typedef virCapsHostNUMACellCPU *virCapsHostNUMACellCPUPtr;
 struct _virCapsHostNUMACellCPU {
 unsigned int id;
+unsigned int capacity;
 unsigned int socket_id;
 unsigned int core_id;
 virBitmapPtr siblings;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 531e0ee..dd2b205 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1175,7 +1175,6 @@ linuxParseCPUmap(int max_cpuid, const char *path)
 return NULL;
 }
 
-
 static virBitmapPtr
 virNodeGetSiblingsList(const char *dir, int cpu_id)
 {
@@ -1873,6 +1872,7 @@ nodeCapsInitNUMAFake(const char *sysfs_prefix,
 if (virNodeGetCpuValue(cpupath, id, "online", 1)) {
 #endif
 cpus[cid].id = id;
+cpus[cid].capacity = 1;
 cpus[cid].socket_id = s;
 cpus[cid].core_id = c;
 if (!(cpus[cid].siblings = virBitmapNew(ncpus)))
@@ -1999,7 +1999,8 @@ nodeGetMemoryFake(unsigned long long *mem,
 static int
 virNodeCapsFillCPUInfo(const char *cpupath ATTRIBUTE_UNUSED,
int cpu_id ATTRIBUTE_UNUSED,
-   virCapsHostNUMACellCPUPtr cpu ATTRIBUTE_UNUSED)
+   virCapsHostNUMACellCPUPtr cpu ATTRIBUTE_UNUSED,
+   int threads_per_subcore)
 {
 #ifdef __linux__
 int tmp;
@@ -2017,9 +2018,24 @@ virNodeCapsFillCPUInfo(const char *cpupath 
ATTRIBUTE_UNUSED,
 
 cpu->core_id = tmp;
 
-if (!(cpu->siblings = virNodeGetSiblingsList(cpupath, cpu_id)))
+cpu->capacity = 1;
+
+if (!threads_per_subcore &&
+!(cpu->siblings = virNodeGetSiblingsList(cpupath, cpu_id)))
 return -1;
 
+/* The primary thread which is online acts in the capacity of whole core
+ * and all its thread siblings. So, if at all one wants to pin the siblings
+ * for efficiency, its better to pin the same primary thread than a
+ * primary thread from other subcore. So, show the siblings as self.
+ */
+if (threads_per_subcore) {
+cpu->capacity = threads_per_subcore;
+if ((cpu->siblings = virBitmapNew(virNumaGetMaxCPUs())) == 0 ||
+virBitmapSetBit(cpu->siblings, cpu_id) < 0)
+return -1;
+}
+
 return 0;
 #else
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
@@ -2123,6 +2139,7 @@ nodeCapsInitNUMA(const char *sysfs_prefix,
 int cpu;
 bool topology_failed = 

[libvirt] [RFC PATCH 1/2] nodeinfo: Reflect guest usable host topology on PPC64

[Shivaprasad G Bhat]

Today, the topology displayed by nodeinfo/capabilities is reflecting the host
topology irrespective the subcore_per_core setting.
This is not of much use as user cannot determine what is the maximum
possible threads_per_core value he can use for a guest topology.

For subcore_per_core = 1, the guest topology can have threads_per_core=1,2,4
or 8. Same way for subcore_per_core = 2, the guest toplogy can have
threads_per_core=1,2 or 4; and for subcore_per_core = 4, guest threads_per_core
can be 1 or 2 only.

Incidentally, the topology displayed in subcore_per_core=1 is correct and is
usable today. If we can display the guest usable topology info for
subcore_per_core=2 and 4 too, then it can help making guest topology decisions.

Signed-off-by: Shivaprasad G Bhat 
---
 src/nodeinfo.c |   17 +
 1 file changed, 17 insertions(+)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 77ea155..531e0ee 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -849,6 +849,23 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 goto cleanup;
 }
 
+/* On PPC if host is in a valid sub core mode, the maximum supported
+ * threads_per_core for a guest is decided by the maximum
+ * threads_per_subcore.
+ * In other words on P8, if the subcores-per-core=1,
+ * the guest can have cpu topology with threads_per_core=1, 2, 4 or 8
+ * and for subcores-per-core=2, the guest can have
+ * threads_per_core=1, 2 or 4 only. Same way, for subcores-per-core=4
+ * guest can have threads_per_core=1 or 2 only.
+ * On a valid kvm host reflecddt the guest supportable topology instead
+ * of host topology.
+ */
+if (threads_per_subcore) {
+int subcores_per_core = nodeinfo->threads/threads_per_subcore;
+nodeinfo->cores *= subcores_per_core;
+nodeinfo->threads /= subcores_per_core;
+}
+
 /* Now check if the topology makes sense. There are machines that don't
  * expose their real number of nodes or for example the AMD Bulldozer
  * architecture that exposes their Clustered integer core modules as both

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

Re: [libvirt] [PATCH 1/4] vz: make output arguments in prlsdkGetDomainIds as optional

[Nikolay Shirokovskiy]

On 28.01.2016 18:38, Mikhail Feoktistov wrote:
> prlsdkGetDomainIds() returns name and uuid for specified instance.
> Now output arguments can be NULL.
> It allows to get only necessary info(name or uuid).
> ---
>  src/vz/vz_sdk.c | 34 +++---
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 7973d6a..8181149 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -346,25 +346,29 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
>  PRL_UINT32 len;
>  PRL_RESULT pret;
>  
> -len = 0;
> -/* get name length */
> -pret = PrlVmCfg_GetName(sdkdom, NULL, );
> -prlsdkCheckRetGoto(pret, error);
> +if (name) {
> +len = 0;
> +/* get name length */
> +pret = PrlVmCfg_GetName(sdkdom, NULL, );
> +prlsdkCheckRetGoto(pret, error);
>  
> -if (VIR_ALLOC_N(*name, len) < 0)
> -goto error;
> +if (VIR_ALLOC_N(*name, len) < 0)
> +goto error;
>  
> -PrlVmCfg_GetName(sdkdom, *name, );
> -prlsdkCheckRetGoto(pret, error);
> +PrlVmCfg_GetName(sdkdom, *name, );
> +prlsdkCheckRetGoto(pret, error);
> +}
>  
> -len = sizeof(uuidstr);
> -PrlVmCfg_GetUuid(sdkdom, uuidstr, );
> -prlsdkCheckRetGoto(pret, error);
> +if (uuid) {
> +len = sizeof(uuidstr);
> +PrlVmCfg_GetUuid(sdkdom, uuidstr, );
> +prlsdkCheckRetGoto(pret, error);
>  
> -if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Domain UUID is malformed or empty"));
> -goto error;
> +if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Domain UUID is malformed or empty"));
> +goto error;
> +}
>  }
>  
>  return 0;
>
ACK, but I would add *name = NULL as another patch or we depend on 
caller setting it for us if we follow error path.

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

Re: [libvirt] [PATCH 6/8] perf: reenable perf events when libvirtd restart

[Ren, Qiaowei]

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, January 28, 2016 11:21 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Jiri Denemark
> Subject: Re: [PATCH 6/8] perf: reenable perf events when libvirtd restart
> 
> On Thu, Dec 10, 2015 at 08:34:36PM +0800, Qiaowei Ren wrote:
> > When libvirtd daemon restart, this patch will reenable those perf
> > events previously enabled.
> >
> > Signed-off-by: Qiaowei Ren 
> > ---
> >  src/qemu/qemu_driver.c | 36 
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
> > 7d95364..09607ca 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -164,6 +164,9 @@ static int qemuDomainGetMaxVcpus(virDomainPtr
> > dom);  static int qemuDomainManagedSaveLoad(virDomainObjPtr vm,
> >   void *opaque);
> >
> > +static int qemuDomainPerfRestart(virDomainObjPtr vm,
> > + void *data);
> > +
> >  static int qemuOpenFile(virQEMUDriverPtr driver,
> >  virDomainObjPtr vm,
> >  const char *path, int oflags, @@ -939,6
> > +942,10 @@ qemuStateInitialize(bool privileged,
> >  qemuDomainManagedSaveLoad,
> >  qemu_driver);
> >
> > +virDomainObjListForEach(qemu_driver->domains,
> > +qemuDomainPerfRestart,
> > +NULL);
> > +
> >  qemuProcessReconnectAll(conn, qemu_driver);
> >
> >  qemu_driver->workerPool = virThreadPoolNew(0, 1, 0,
> > qemuProcessEventHandler, qemu_driver); @@ -3460,6 +3467,35 @@
> qemuDomainManagedSaveLoad(virDomainObjPtr vm,
> >  return ret;
> >  }
> >
> > +static int
> > +qemuDomainPerfRestart(virDomainObjPtr vm,
> > +  void *data ATTRIBUTE_UNUSED) {
> > +size_t i;
> > +virDomainDefPtr def = vm->def;
> > +qemuDomainObjPrivatePtr priv = vm->privateData;
> > +
> > +virPerfFree(priv->perf);
> > +
> > +priv->perf = virPerfNew();
> > +if (!priv->perf)
> > +return -1;
> > +
> > +for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
> > +if (def->perf->events[i] &&
> > +def->perf->events[i] == VIR_TRISTATE_BOOL_YES) {
> > +if (virPerfEventEnable(priv->perf, i, vm->pid))
> > +goto cleanup;
> > +}
> > +}
> > +
> > +return 0;
> > +
> > + cleanup:
> > +virPerfFree(priv->perf);
> > +return -1;
> > +}
> > +
> 
> Oh, I suggested we add this in qemuProcessReconnect in a previous patch
> 

Yes. I will do this in  qemuProcessReconnect and qemuProcessAttach according to 
the comments in patch 4/8, but because this patch have to depend on the new XML 
element (patch 5/8), and so I have to put these code into the patch and could 
not merge it into the patch 4/8. ^-^

Thanks,
Qiaowei

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

[libvirt] [PATCH 2/4] vz: remove unused struct field

[Mikhail Feoktistov]

In commit 7039bb3c we have removed code that saves uuid to vzDomObj.uuid
So this field is no longer needed.
---
 src/vz/vz_sdk.c   | 5 -
 src/vz/vz_utils.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 8181149..40ab2d9 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -417,7 +417,6 @@ prlsdkDomObjFreePrivate(void *p)
 PrlHandle_Free(pdom->sdkdom);
 PrlHandle_Free(pdom->cache.stats);
 virCondDestroy(>cache.cond);
-VIR_FREE(pdom->uuid);
 VIR_FREE(pdom->home);
 VIR_FREE(p);
 };
@@ -1282,10 +1281,6 @@ prlsdkLoadDomain(vzConnPtr privconn,
 
 def->id = -1;
 
-/* we will remove this field in the near future, so let's set it
- * to NULL temporarily */
-pdom->uuid = NULL;
-
 pdom->cache.stats = PRL_INVALID_HANDLE;
 pdom->cache.count = -1;
 if (virCondInit(>cache.cond) < 0) {
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index 84cf08f..0fbbfc7 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -75,7 +75,6 @@ typedef struct _vzCountersCache vzCountersCache;
 
 struct vzDomObj {
 int id;
-char *uuid;
 char *home;
 PRL_HANDLE sdkdom;
 vzCountersCache cache;
-- 
1.8.3.1

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

[libvirt] [PATCH 4/4] vz: fix race condition when adding domain to domains list

[Mikhail Feoktistov]

Race condition:
User calls defineXML to create new instance.
The main thread from vzDomainDefineXMLFlags() creates new instance by 
prlsdkCreateVm.
Then this thread calls prlsdkAddDomain to add new domain to domains list.
The second thread receives notification from hypervisor that new VM was created.
It calls prlsdkHandleVmAddedEvent() and also tries to add new domain to domains 
list.
These two threads call virDomainObjListFindByUUID() from prlsdkAddDomain() and 
don't find new domain.
So they add two domains with the same uuid to domains list.

This fix splits logic of prlsdkAddDomain() into two functions.
1. prlsdkNewDomain() creates new empty domain in domains list with the specific 
uuid.
2. prlsdkLoadDomain() add data from VM to domain object.

New algorithm for creating an instance:
In vzDomainDefineXMLFlags() we add new domain to domain list by calling 
prlsdkNewDomain()
and only after that we call CreateVm() to create VM.
It means that we "reserve" domain object with the specific uuid.
After creation of new VM we add info from this VM
to reserved domain object by calling prlsdkLoadDomain().

Before this patch prlsdkLoadDomain() worked in 2 different cases:
1. It creates and initializes new domain. Then updates it from sdk handle.
2. It updates existed domain from sdk handle.
In this patch we remove code which creates new domain from LoadDomain()
and move it to prlsdkNewDomain().
Now prlsdkLoadDomain() only updates domain from skd handle.

In notification handler prlsdkHandleVmAddedEvent() we check
the existence of a domain and if it doesn't exist we add new domain by calling
prlsdkNewDomain() and load info from sdk handle via prlsdkLoadDomain().
---
 src/vz/vz_driver.c |  13 ++-
 src/vz/vz_sdk.c| 253 +++--
 src/vz/vz_sdk.h|   9 +-
 3 files changed, 146 insertions(+), 129 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 91a48b6..521efd4 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -685,6 +685,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, 
unsigned int flags)
 virDomainPtr retdom = NULL;
 virDomainDefPtr def;
 virDomainObjPtr olddom = NULL;
+virDomainObjPtr newdom = NULL;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
 
 virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);
@@ -700,6 +701,9 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, 
unsigned int flags)
 olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid);
 if (olddom == NULL) {
 virResetLastError();
+newdom = prlsdkNewDomain(privconn, def->name, def->uuid);
+if (!newdom)
+goto cleanup;
 if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
 if (prlsdkCreateVm(conn, def))
 goto cleanup;
@@ -713,8 +717,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, 
unsigned int flags)
 goto cleanup;
 }
 
-olddom = prlsdkAddDomain(privconn, def->uuid);
-if (!olddom)
+if (prlsdkLoadDomain(privconn, newdom))
 goto cleanup;
 } else {
 int state, reason;
@@ -755,6 +758,12 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char 
*xml, unsigned int flags)
  cleanup:
 if (olddom)
 virObjectUnlock(olddom);
+if (newdom) {
+if (!retdom)
+ virDomainObjListRemove(privconn->domains, newdom);
+else
+ virObjectUnlock(newdom);
+}
 virDomainDefFree(def);
 vzDriverUnlock(privconn);
 return retdom;
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 6fb2a97..9d2bdab 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1236,58 +1236,35 @@ prlsdkConvertCpuMode(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
 return -1;
 }
 
-/*
- * This function retrieves information about domain.
- * If the domains is already in the domains list
- * privconn->domains, then locked 'olddom' must be
- * provided. If the domains must be added to the list,
- * olddom must be NULL.
- *
- * The function return a pointer to a locked virDomainObj.
- */
-static virDomainObjPtr
-prlsdkLoadDomain(vzConnPtr privconn,
- PRL_HANDLE sdkdom,
- virDomainObjPtr olddom)
+int
+prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom)
 {
-virDomainObjPtr dom = NULL;
 virDomainDefPtr def = NULL;
-vzDomObjPtr pdom = NULL;
 VIRTUAL_MACHINE_STATE domainState;
+char *home = NULL;
 
 PRL_UINT32 buflen = 0;
 PRL_RESULT pret;
 PRL_UINT32 ram;
 PRL_UINT32 envId;
 PRL_VM_AUTOSTART_OPTION autostart;
+PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
+vzDomObjPtr pdom = dom->privateData;
 
 virCheckNonNullArgGoto(privconn, error);
-virCheckNonNullArgGoto(sdkdom, error);
+virCheckNonNullArgGoto(dom, error);
+
+sdkdom = prlsdkSdkDomainLookupByUUID(privconn, dom->def->uuid);
+if (sdkdom == PRL_INVALID_HANDLE)
+return -1;
 
 if (!(def = 

[libvirt] [PATCH 0/4 v2] vz: rework domain creation

[Mikhail Feoktistov]

Patches 1 and 2 make preparation for patch 4

Patch 3 fixes notification subscription
To get domain info we should receive and handle notification from hypervisor

Patch 4 fixes race condition when adding domain to domains list
Race condition:
User calls defineXML to create new instance.
The main thread from vzDomainDefineXMLFlags() creates new instance by 
prlsdkCreateVm.
Then this thread calls prlsdkAddDomain to add new domain to domains list.
The second thread receives notification from hypervisor that new VM was created.
It calls prlsdkHandleVmAddedEvent() and also tries to add new domain to domains 
list.
These two threads call virDomainObjListFindByUUID() from prlsdkAddDomain() and 
don't find new domain.
So they add two domains with the same uuid to domains list.

Mikhail Feoktistov (4):
  vz: make output arguments in prlsdkGetDomainIds as optional
  vz: remove unused struct field
  vz: fix notification subscription
  vz: fix race condition when adding domain to domains list

 src/vz/vz_driver.c |  13 ++-
 src/vz/vz_sdk.c| 293 +++--
 src/vz/vz_sdk.h|   9 +-
 src/vz/vz_utils.h  |   1 -
 4 files changed, 165 insertions(+), 151 deletions(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 3/4] vz: fix notification subscription

[Mikhail Feoktistov]

Bug cause:
Update the domain that is subscribed to hypervisor notification.
LoadDomain() rewrites notifications fields in vzDomObj structure and makes 
domain as "unsubscribed".
Fix:
Initialize notification fields in vzDomObj only if we create a new domain.
And do not reinitialize these fields if we update domain (by calling LoadDomain 
with olddom argument)
---
 src/vz/vz_sdk.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 40ab2d9..6fb2a97 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1270,6 +1270,12 @@ prlsdkLoadDomain(vzConnPtr privconn,
 if (!olddom) {
 if (VIR_ALLOC(pdom) < 0)
 goto error;
+pdom->cache.stats = PRL_INVALID_HANDLE;
+pdom->cache.count = -1;
+if (virCondInit(>cache.cond) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize 
condition"));
+goto error;
+}
 } else {
 pdom = olddom->privateData;
 }
@@ -1281,13 +1287,6 @@ prlsdkLoadDomain(vzConnPtr privconn,
 
 def->id = -1;
 
-pdom->cache.stats = PRL_INVALID_HANDLE;
-pdom->cache.count = -1;
-if (virCondInit(>cache.cond) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize 
condition"));
-goto error;
-}
-
 if (prlsdkGetDomainIds(sdkdom, >name, def->uuid) < 0)
 goto error;
 
-- 
1.8.3.1

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

[libvirt] [PATCH v2 3/6] logical: Search for a segtype of "thin" and mark lv as sparse

[John Ferlan]

For any "thin" lv's, mark the volume as a sparse volume so that the
volume wipe algorithm doesn't work.  Currently thin lv's are ignored
because the regex requires 1 or more 'devices' listed in order to
process. However, a future patch will be changing this.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index 3232c08..a9c6309 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -64,6 +64,8 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
 }
 
 
+#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN "thin"
+
 struct virStorageBackendLogicalPoolVolData {
 virStoragePoolObjPtr pool;
 virStorageVolDefPtr vol;
@@ -201,12 +203,15 @@ virStorageBackendLogicalMakeVol(char **const groups,
 }
 
 /* Mark the (s) sparse/snapshot lv, e.g. the lv created using
- * the --virtualsize/-V option. We've already ignored the (t)hin
+ * the --virtualsize/-V option or a thin segtype as sparse. This
+ * will make sure the volume wipe algorithm doesn't overwrite
+ * a sparse/thin volumes. We've already ignored the (t)hin
  * pool definition. In the manner libvirt defines these, the
  * thin pool is hidden to the lvs output, except as the name
  * in brackets [] described for the groups[1] (backingStore).
  */
-if (attrs[0] == 's')
+if (attrs[0] == 's' ||
+STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN))
 vol->target.sparse = true;
 
 /* Skips the backingStore of lv created with "--virtualsize",
-- 
2.5.0

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

[libvirt] [PATCH v2 4/6] vol: Add thin_pool to _virStorageVolSource

[John Ferlan]

A thin lv doesn't have any extents defined - rather it uses a concept of
a "thin-pool" in order to describe it's source. In order to allow that to
be displayed properly in a future patch, add a new 'thin_pool' name that
can be used by a future patch to store the name of the source thin_pool
name used by a thin logical volume.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.c | 5 +
 src/conf/storage_conf.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3657dfd..8ceb465 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -322,6 +322,7 @@ virStorageVolDefFree(virStorageVolDefPtr def)
 for (i = 0; i < def->source.nextent; i++)
 VIR_FREE(def->source.extents[i].path);
 VIR_FREE(def->source.extents);
+VIR_FREE(def->source.thin_pool);
 
 virStorageSourceClear(>target);
 VIR_FREE(def);
@@ -1655,6 +1656,10 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
 virBufferAddLit(, "\n");
 }
 
+if (def->source.thin_pool)
+virBufferEscapeString(, "%s\n",
+  def->source.thin_pool);
+
 virBufferAdjustIndent(, -2);
 virBufferAddLit(, "\n");
 
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index f1dc62b..7e15a70 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -55,6 +55,7 @@ struct _virStorageVolSource {
 
 int partType; /* virStorageVolTypeDisk, only used by disk
* backend for partition type creation */
+char *thin_pool; /* Used to print/dumpxml the thin pool name */
 };
 
 
-- 
2.5.0

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

[libvirt] [PATCH v2 6/6] logical: Display thin lv's found in a libvirt managed volume group

[John Ferlan]

Modify the regex for the 'devices' (a/k/a 'extents') from "(\\S+)"
(e.g., 1 or more) to "(\\S*)" (e.g., zero or more).

Since a "thin" segtype has no devices, this will result in any "thin" lv
part of some thin-pool within a volume group used as a libvirt pool to be
displayed as a possible volume to use.

NB: Based on a proposal authored by Joe Harvell ,
but with much intervening rework, the resulting patch is changed from
the original concept. About all that remains is changing the regex.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index b866648..c56961d 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -81,9 +81,12 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr 
vol,
 size_t i, nextents = 0;
 unsigned long long offset, size, length;
 
+/* If the groups field is NULL, then there's nothing to do */
+if (!groups[3])
+return 0;
+
 /* The 'devices' (or extents) are split by a comma ",", so let's split
- * each out into a parseable string. Since our regex to generate this
- * data is "(\\S+)", we can safely assume at least one exists. */
+ * each out into a parseable string. */
 if (!(extents = virStringSplitCount(groups[3], ",", 0, )))
 goto cleanup;
 
@@ -296,7 +299,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
  *striped, so "," is not a suitable separator either (rhbz 727474).
  */
 const char *regexes[] = {
-   
"^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#(\\S*)#?\\s*$"
+   
"^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S*)#(\\S+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#(\\S*)#?\\s*$"
 };
 int vars[] = {
 10
-- 
2.5.0

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

[libvirt] [PATCH v2 1/6] logical: Create helper virStorageBackendLogicalParseVolExtents

[John Ferlan]

Create a helper routine in order to parse any extents information
including the extent size, length, and the device string contained
within the generated 'lvs' output string.

A future patch would then be able to avoid the code more cleanly

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c | 208 ++
 1 file changed, 113 insertions(+), 95 deletions(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index 944be40..7c05b6a 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -72,98 +72,18 @@ struct virStorageBackendLogicalPoolVolData {
 };
 
 static int
-virStorageBackendLogicalMakeVol(char **const groups,
-void *opaque)
+virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
+char **const groups)
 {
-struct virStorageBackendLogicalPoolVolData *data = opaque;
-virStoragePoolObjPtr pool = data->pool;
-virStorageVolDefPtr vol = NULL;
-bool is_new_vol = false;
-unsigned long long offset, size, length;
+int nextents, ret = -1;
 const char *regex_unit = "(\\S+)\\((\\S+)\\)";
 char *regex = NULL;
 regex_t *reg = NULL;
 regmatch_t *vars = NULL;
 char *p = NULL;
 size_t i;
-int err, nextents, nvars, ret = -1;
-const char *attrs = groups[9];
-
-/* Skip inactive volume */
-if (attrs[4] != 'a')
-return 0;
-
-/*
- * Skip thin pools(t). These show up in normal lvs output
- * but do not have a corresponding /dev/$vg/$lv device that
- * is created by udev. This breaks assumptions in later code.
- */
-if (attrs[0] == 't')
-return 0;
-
-/* See if we're only looking for a specific volume */
-if (data->vol != NULL) {
-vol = data->vol;
-if (STRNEQ(vol->name, groups[0]))
-return 0;
-}
-
-/* Or filling in more data on an existing volume */
-if (vol == NULL)
-vol = virStorageVolDefFindByName(pool, groups[0]);
-
-/* Or a completely new volume */
-if (vol == NULL) {
-if (VIR_ALLOC(vol) < 0)
-return -1;
-
-is_new_vol = true;
-vol->type = VIR_STORAGE_VOL_BLOCK;
-
-if (VIR_STRDUP(vol->name, groups[0]) < 0)
-goto cleanup;
-
-}
-
-if (vol->target.path == NULL) {
-if (virAsprintf(>target.path, "%s/%s",
-pool->def->target.path, vol->name) < 0)
-goto cleanup;
-}
-
-/* Mark the (s) sparse/snapshot lv, e.g. the lv created using
- * the --virtualsize/-V option. We've already ignored the (t)hin
- * pool definition. In the manner libvirt defines these, the
- * thin pool is hidden to the lvs output, except as the name
- * in brackets [] described for the groups[1] (backingStore).
- */
-if (attrs[0] == 's')
-vol->target.sparse = true;
-
-/* Skips the backingStore of lv created with "--virtualsize",
- * its original device "/dev/$vgname/$lvname_vorigin" is
- * just for lvm internal use, one should never use it.
- *
- * (lvs outputs "[$lvname_vorigin] for field "origin" if the
- *  lv is created with "--virtualsize").
- */
-if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
-if (VIR_ALLOC(vol->target.backingStore) < 0)
-goto cleanup;
-
-if (virAsprintf(>target.backingStore->path, "%s/%s",
-pool->def->target.path, groups[1]) < 0)
-goto cleanup;
-
-vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
-}
-
-if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
-goto cleanup;
-
-if (virStorageBackendUpdateVolInfo(vol, false,
-   VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
-goto cleanup;
+int err, nvars;
+unsigned long long offset, size, length;
 
 nextents = 1;
 if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED)) {
@@ -174,7 +94,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
 }
 }
 
-/* Finally fill in extents information */
+/* Allocate and fill in extents information */
 if (VIR_REALLOC_N(vol->source.extents,
   vol->source.nextent + nextents) < 0)
 goto cleanup;
@@ -184,18 +104,13 @@ virStorageBackendLogicalMakeVol(char **const groups,
"%s", _("malformed volume extent length value"));
 goto cleanup;
 }
+
 if (virStrToLong_ull(groups[7], NULL, 10, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume extent size value"));
 goto cleanup;
 }
-if (virStrToLong_ull(groups[8], NULL, 10, >target.allocation) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("malformed volume allocation 

[libvirt] [PATCH v2 5/6] logical: Add capability to get the thin pool name

[John Ferlan]

Since a future patch will start displaying thin logical volumes for a
logical volume group thin-pool, add the ability to display at the name of
the thin-pool name. This is done by adding 'pool_lv' to the list of
regex's and then when a "thin" lv is found, storing that pool name.

The result will end up being the following for a vol-dumpxml:

  
thinpool_test_thin
  

instead of an empty   pair.

Signed-off-by: John Ferlan 
---
 docs/formatstorage.html.in|  9 +++--
 src/storage/storage_backend_logical.c | 26 --
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 4965a4c..2f4662c 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -566,8 +566,13 @@
 Since 0.4.1
   source
   Provides information about the underlying storage allocation
-of the volume. This may not be available for some pool types.
-Since 0.4.1
+of the volume. This is available logical pool types to display
+details of logical volume extent information
+(Since 0.4.1)
+or the name of the name of the thin-pool used by the thin
+logical volume
+(Since 1.3.2).
+
   target
   Provides information about the representation of the volume
 on the local host. Since 0.4.1
diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index a9c6309..b866648 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -239,6 +239,11 @@ virStorageBackendLogicalMakeVol(char **const groups,
VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
 goto cleanup;
 
+if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN)) {
+if (VIR_STRDUP(vol->source.thin_pool, groups[9]) < 0)
+goto cleanup;
+}
+
 if (virStrToLong_ull(groups[7], NULL, 10, >target.allocation) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume allocation value"));
@@ -266,14 +271,15 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
 {
 /*
  * # lvs --separator # --noheadings --units b --unbuffered --nosuffix 
--options \
- * 
"lv_name,origin,uuid,devices,segtype,seg_size,vg_extent_size,size,lv_attr" 
VGNAME
+ * 
"lv_name,origin,uuid,devices,segtype,seg_size,vg_extent_size,size,lv_attr,pool_lv"
 VGNAME
  *
- * 
RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#linear#5234491392#33554432#5234491392#-wi-ao
- * 
SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#linear#1040187392#33554432#1040187392#-wi-ao
- * 
Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#linear#1073741824#33554432#1073741824#owi-a-
- * 
Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#linear#2181038080#33554432#2181038080#-wi-a-
- * 
Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#linear#1040187392#33554432#1040187392#swi-a-
- * 
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#/dev/sdc1(10240),/dev/sdd1(0)#striped#42949672960#4194304#-wi-a-
+ * 
RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#linear#5234491392#33554432#5234491392#-wi-ao#
+ * 
SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#linear#1040187392#33554432#1040187392#-wi-ao#
+ * 
Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#linear#1073741824#33554432#1073741824#owi-a-#
+ * 
Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#linear#2181038080#33554432#2181038080#-wi-a-#
+ * 
Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#linear#1040187392#33554432#1040187392#swi-a-#
+ * 
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#/dev/sdc1(10240),/dev/sdd1(0)#striped#42949672960#4194304#-wi-a-#
+ * 
test_thin##9cdaL5-qEyd-pbRn-Ef2B-uU81-WSIE-XUY973##thin#41943040#4194304#41943040#Vwi-a-tz--#thinpool_test_thin
  *
  * Pull out name, origin, & uuid, device, device extent start #,
  * segment size, extent size, size, attrs
@@ -290,10 +296,10 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
  *striped, so "," is not a suitable separator either (rhbz 727474).
  */
 const char *regexes[] = {
-   
"^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
+   
"^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#(\\S*)#?\\s*$"
 };
 int vars[] = {
-9
+10
 };
 int ret = -1;
 virCommandPtr cmd;
@@ -309,7 +315,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
"--unbuffered",
"--nosuffix",
"--options",
-   

[libvirt] [PATCH v2 2/6] logical: Fix parsing of the 'devices' field lvs output

[John Ferlan]

Amongst many other changes, commit id '82c1740a' added a display of the
lvs -o 'stripes' field in order to be able to determine the number of
extents listed in the 'devices' output. This was then used to generate
a regex in order to parse the devices string. As part of the generation
of that regex and various other allocations was adding a comma ","
separator for each regex. In essance quite a bit of code in order to
parse a comma separated set of strings having a specific format.

Furthermore, the 'stripes' field is described as the "Number of stripes
or mirror legs"; however, the setting of the 'nextents' value to something
other than 1 was masked behind comparing the output of the lvs -o 'segtype'
field to "striped". Thus, for a "mirror" volume (or today for raid segtypes)
the code would assume only 1 extent (or device) in the list.  This would
lead to output in a vol-dumpxml of:

  

  

   

This patch removes all the regex/regcomp logic in favor of a simpler
mechanism of splitting the devices (groups[3]) output by the comma ","
separator. Then once split, each output string is further parsed
extracting out the parenthesized starting extent number. This is
then all stored in the volume extents array.

The resulting mirror output is then:

  

  


  

  

Although used now, the 'segtype' field is kept for usage by a
future patch.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c | 149 --
 tests/virstringtest.c |  11 +++
 2 files changed, 62 insertions(+), 98 deletions(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index 7c05b6a..3232c08 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -64,8 +64,6 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
 }
 
 
-#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED "striped"
-
 struct virStorageBackendLogicalPoolVolData {
 virStoragePoolObjPtr pool;
 virStorageVolDefPtr vol;
@@ -75,121 +73,76 @@ static int
 virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
 char **const groups)
 {
-int nextents, ret = -1;
-const char *regex_unit = "(\\S+)\\((\\S+)\\)";
-char *regex = NULL;
-regex_t *reg = NULL;
-regmatch_t *vars = NULL;
-char *p = NULL;
-size_t i;
-int err, nvars;
+int ret = -1;
+char **extents = NULL;
+char *extnum, *end;
+size_t i, nextents = 0;
 unsigned long long offset, size, length;
 
-nextents = 1;
-if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED)) {
-if (virStrToLong_i(groups[5], NULL, 10, ) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("malformed volume extent stripes value"));
-goto cleanup;
-}
-}
+/* The 'devices' (or extents) are split by a comma ",", so let's split
+ * each out into a parseable string. Since our regex to generate this
+ * data is "(\\S+)", we can safely assume at least one exists. */
+if (!(extents = virStringSplitCount(groups[3], ",", 0, )))
+goto cleanup;
 
 /* Allocate and fill in extents information */
 if (VIR_REALLOC_N(vol->source.extents,
   vol->source.nextent + nextents) < 0)
 goto cleanup;
+vol->source.nextent += nextents;
 
-if (virStrToLong_ull(groups[6], NULL, 10, ) < 0) {
+if (virStrToLong_ull(groups[5], NULL, 10, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume extent length value"));
 goto cleanup;
 }
 
-if (virStrToLong_ull(groups[7], NULL, 10, ) < 0) {
+if (virStrToLong_ull(groups[6], NULL, 10, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume extent size value"));
 goto cleanup;
 }
 
-if (VIR_STRDUP(regex, regex_unit) < 0)
-goto cleanup;
-
-for (i = 1; i < nextents; i++) {
-if (VIR_REALLOC_N(regex, strlen(regex) + strlen(regex_unit) + 2) < 0)
-goto cleanup;
-/* "," is the separator of "devices" field */
-strcat(regex, ",");
-strncat(regex, regex_unit, strlen(regex_unit));
-}
-
-if (VIR_ALLOC(reg) < 0)
-goto cleanup;
-
-/* Each extent has a "path:offset" pair, and vars[0] will
- * be the whole matched string.
+/* The lvs 'devices' field is described as "Underlying devices used
+ * with starting extent numbers." - The format is a "path" element
+ * followed by a "(#)", such as:
+ *
+ */dev/hda2(0) <--- linear segtype
+ */dev/sdc1(10240),/dev/sdd1(0)<--- striped segtype
+ *test_mirror_rimage_0(0),test_mirror_rimage_1(0)   <-- mirror/raid
+ *  segtype
+

[libvirt] [PATCH v2 0/6] Some logical pool/volume changes

[John Ferlan]

v1: http://www.redhat.com/archives/libvir-list/2016-January/msg01023.html

Differences to v1:

Patch 1 is the former patch 2 with the following adjuments:
 - Change helper name to be virStorageBackendLogicalParseVolExtents
 - Include all 'extent' parsing needs - including fetch/parse of 'stripes'
   to determine 'nextents' value, allocation of vol->source.extents,
   parse of groups[6] to be 'length', parse of groups[7] to be 'size'

Patch 2 is new, but replaces some of the concepts from patch 3:
 - I rototilled the virStorageBackendLogicalParseVolExtents to remove
   the unnecessary trip through regex/regcomp. 
 - Used virStringSplitCount instread and then parse the resultant string.
 - Removed the need for the 'stripes' fetch since it really wasn't doing
   what it was supposed to.
 - I adjusted the virstringtest to exhibit what is being parsed (it can
   be removed if you think that's excessive).

Patches 3-5 are new
 - These could be combined. I left them separate to show the process...
 - Essentially adding a new "source" field to be used to display in
   the vol-dumpxml output

Patch 6 is part of the old patch 4:
 - Since the old patch 3 isn't necessary - it's essentially gone. This
   just takes the patch 4 concept of changing the regex, but also applies
   it to the current code.

John Ferlan (6):
  logical: Create helper virStorageBackendLogicalParseVolExtents
  logical: Fix parsing of the 'devices' field lvs output
  logical: Search for a segtype of "thin" and mark lv as sparse
  vol: Add thin_pool to _virStorageVolSource
  logical: Add capability to get the thin pool name
  logical: Display thin lv's found in a libvirt managed volume group

 docs/formatstorage.html.in|   9 +-
 src/conf/storage_conf.c   |   5 +
 src/conf/storage_conf.h   |   1 +
 src/storage/storage_backend_logical.c | 221 --
 tests/virstringtest.c |  11 ++
 5 files changed, 127 insertions(+), 120 deletions(-)

-- 
2.5.0

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

[libvirt] [PATCH] qemu: Mark some functions as static

[Cole Robinson]

---
 src/qemu/qemu_command.c | 16 +---
 src/qemu/qemu_command.h | 11 ---
 src/qemu/qemu_process.h |  2 --
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5d3ab3a..8943270 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1775,6 +1775,16 @@ qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr 
bus)
 }
 
 
+static int
+qemuAssignDevicePCISlots(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ virDomainPCIAddressSetPtr addrs);
+static int
+qemuDomainAssignPCIAddresses(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ virDomainObjPtr obj);
+
+
 int qemuDomainAssignAddresses(virDomainDefPtr def,
   virQEMUCapsPtr qemuCaps,
   virDomainObjPtr obj)
@@ -1801,7 +1811,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def,
 }
 
 
-virDomainPCIAddressSetPtr
+static virDomainPCIAddressSetPtr
 qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
   unsigned int nbuses,
   bool dryRun)
@@ -2238,7 +2248,7 @@ qemuValidateDevicePCISlotsChipsets(virDomainDefPtr def,
 return 0;
 }
 
-int
+static int
 qemuDomainAssignPCIAddresses(virDomainDefPtr def,
  virQEMUCapsPtr qemuCaps,
  virDomainObjPtr obj)
@@ -2451,7 +2461,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
  * function must only try to reserve addresses if info.type == NONE and
  * skip over info.type == PCI
  */
-int
+static int
 qemuAssignDevicePCISlots(virDomainDefPtr def,
  virQEMUCapsPtr qemuCaps,
  virDomainPCIAddressSetPtr addrs)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index a96e57d..53bfda5 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -274,17 +274,6 @@ void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
 const char *devstr);
 
 
-int qemuDomainAssignPCIAddresses(virDomainDefPtr def,
- virQEMUCapsPtr qemuCaps,
- virDomainObjPtr obj);
-virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
-unsigned int nbuses,
-bool dryRun);
-
-int qemuAssignDevicePCISlots(virDomainDefPtr def,
- virQEMUCapsPtr qemuCaps,
- virDomainPCIAddressSetPtr addrs);
-
 int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
 int qemuDomainNetVLAN(virDomainNetDefPtr def);
 int qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int 
idx);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c674111..cb5cee1 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -42,8 +42,6 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
 void qemuProcessAutostartAll(virQEMUDriverPtr driver);
 void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver);
 
-int qemuProcessAssignPCIAddresses(virDomainDefPtr def);
-
 typedef struct _qemuProcessIncomingDef qemuProcessIncomingDef;
 typedef qemuProcessIncomingDef *qemuProcessIncomingDefPtr;
 struct _qemuProcessIncomingDef {
-- 
2.5.0

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

[libvirt] [PATCH 6/9] tests: qemuxml2xml: Allow test cases to pass in qemuCaps

[Cole Robinson]

Similar to how we do it for qemuxml2argvtest. This will be used in future
patches.
---
 tests/qemuxml2xmltest.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2400538..0134815 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -250,12 +250,13 @@ mymain(void)
 /* TODO: test with format probing disabled too */
 driver.config->allowDiskFormatProbing = true;
 
-# define DO_TEST_FULL(name, when)  
\
+# define DO_TEST_FULL(name, when, ...)\
 do {   
\
 if (testInfoSet(, name, when) < 0) { \
 VIR_TEST_DEBUG("Failed to generate test data for '%s'", name);\
 return -1; 
\
 }  
\
+virQEMUCapsSetList(info.qemuCaps, __VA_ARGS__, QEMU_CAPS_LAST);
\

\
 if (info.outInactiveName) {
\
 if (virtTestRun("QEMU XML-2-XML-inactive " name,   
\
@@ -275,8 +276,12 @@ mymain(void)
 testInfoFree();   
\
 } while (0)
 
+# define NONE QEMU_CAPS_LAST
+
 # define DO_TEST(name) \
-DO_TEST_FULL(name, WHEN_BOTH)
+DO_TEST_FULL(name, WHEN_BOTH, NONE)
+
+
 
 /* Unset or set all envvars here that are copied in qemudBuildCommandLine
  * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
@@ -381,9 +386,9 @@ mymain(void)
 DO_TEST("disk-virtio-scsi-ioeventfd");
 DO_TEST("disk-scsi-megasas");
 DO_TEST("disk-mirror-old");
-DO_TEST_FULL("disk-mirror", WHEN_ACTIVE);
-DO_TEST_FULL("disk-mirror", WHEN_INACTIVE);
-DO_TEST_FULL("disk-active-commit", WHEN_ACTIVE);
+DO_TEST_FULL("disk-mirror", WHEN_ACTIVE, NONE);
+DO_TEST_FULL("disk-mirror", WHEN_INACTIVE, NONE);
+DO_TEST_FULL("disk-active-commit", WHEN_ACTIVE, NONE);
 DO_TEST("graphics-listen-network");
 DO_TEST("graphics-vnc");
 DO_TEST("graphics-vnc-websocket");
@@ -478,17 +483,17 @@ mymain(void)
 DO_TEST("blkdeviotune");
 DO_TEST("controller-usb-order");
 
-DO_TEST_FULL("seclabel-dynamic-baselabel", WHEN_INACTIVE);
-DO_TEST_FULL("seclabel-dynamic-override", WHEN_INACTIVE);
-DO_TEST_FULL("seclabel-dynamic-labelskip", WHEN_INACTIVE);
-DO_TEST_FULL("seclabel-dynamic-relabel", WHEN_INACTIVE);
+DO_TEST_FULL("seclabel-dynamic-baselabel", WHEN_INACTIVE, NONE);
+DO_TEST_FULL("seclabel-dynamic-override", WHEN_INACTIVE, NONE);
+DO_TEST_FULL("seclabel-dynamic-labelskip", WHEN_INACTIVE, NONE);
+DO_TEST_FULL("seclabel-dynamic-relabel", WHEN_INACTIVE, NONE);
 DO_TEST("seclabel-static");
-DO_TEST_FULL("seclabel-static-labelskip", WHEN_ACTIVE);
+DO_TEST_FULL("seclabel-static-labelskip", WHEN_ACTIVE, NONE);
 DO_TEST("seclabel-none");
 DO_TEST("seclabel-dac-none");
 DO_TEST("seclabel-dynamic-none");
 DO_TEST("seclabel-device-multiple");
-DO_TEST_FULL("seclabel-dynamic-none-relabel", WHEN_INACTIVE);
+DO_TEST_FULL("seclabel-dynamic-none-relabel", WHEN_INACTIVE, NONE);
 DO_TEST("numad-static-vcpu-no-numatune");
 DO_TEST("disk-scsi-lun-passthrough-sgio");
 
-- 
2.5.0

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

Re: [libvirt] [PATCH v2 7/8] qemu: hotplug: Drop !QEMU_CAPS_DEVICE code

[Cole Robinson]

On 01/26/2016 11:44 AM, Laine Stump wrote:
> On 01/22/2016 02:11 PM, Cole Robinson wrote:
>> On 01/22/2016 02:09 PM, Cole Robinson wrote:
>>> Nowadays we only support qemu 0.12.0+ which provides QEMU_CAPS_DEVICE,
>>> so this is all dead code.
>>> ---
>>>   src/qemu/qemu_hotplug.c | 480
>>> +++-
>>>   1 file changed, 144 insertions(+), 336 deletions(-)
>>>
>> git show -w attached
>>
> 
> Just nitpicking, but seeing it this way allowed me to randomly notice that the
> changes have created several instances of code like this:
> 
> if (!detach->info.alias) {
> if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, detach) 
> < 0)
> goto cleanup;
> }
> 
> Maybe combine those like this:
> 
> if (!detach->info.alias &&
> (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, detach) < 
> 0))
> goto cleanup;
> 
> 
> 

Thanks, fixed locally. It will be in v2 whenever I get around to posting it.

Thanks,
Cole

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

Re: [libvirt] [PATCH v2 0/8] qemu: Drop QEMU_CAPS_DEVICE part 1

[Cole Robinson]

On 01/22/2016 02:09 PM, Cole Robinson wrote:
> libvirt only supports qemu 0.12.0+ nowadays, which means that the
> qemu binary always provides the -device/QEMU_CAPS_DEVICE option. This
> patch series is a step towards dropping support for that flag.
> 
> Patch #1 is a test suite improvement, not strictly related
> Patch #2-5 adjust the test suite for -device always being present
> Remaining patches drop a bunch of qemu driver code.
> 
> After this the only real remaining uses are in qemu_command.c which
> deserve a closer inspection, so that will be a separate series.

I'm concentrating on getting the test changes committed first, so please
ignore the qemu side of this. I'll repost eventually

- Cole

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

[libvirt] [PATCH 7/9] tests: utils: Add PreFormat callback for CompareXML2XML helper

[Cole Robinson]

This allows individual driver tests to hook in their own code before
the def is formatted and compared.

We will eventually use this in the qemuxml2xml
---
 tests/bhyvexml2xmltest.c   | 3 ++-
 tests/genericxml2xmltest.c | 3 ++-
 tests/lxcxml2xmltest.c | 3 ++-
 tests/qemuxml2xmltest.c| 6 --
 tests/testutils.c  | 7 ++-
 tests/testutils.h  | 6 +-
 6 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index d860a41..2d34a00 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -32,7 +32,8 @@ testCompareXMLToXMLHelper(const void *data)
 
 ret = testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, xml_in,
  info->different ? xml_out : xml_in,
- false);
+ false,
+ NULL, NULL);
 
  cleanup:
 VIR_FREE(xml_in);
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index 7f79896..aa3a570 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -39,7 +39,8 @@ testCompareXMLToXMLHelper(const void *data)
 
 ret = testCompareDomXML2XMLFiles(caps, xmlopt, xml_in,
  info->different ? xml_out : xml_in,
- !info->inactive_only);
+ !info->inactive_only,
+ NULL, NULL);
  cleanup:
 VIR_FREE(xml_in);
 VIR_FREE(xml_out);
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index e460d0a..aafebb4 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -44,7 +44,8 @@ testCompareXMLToXMLHelper(const void *data)
 
 ret = testCompareDomXML2XMLFiles(caps, xmlopt, xml_in,
  info->different ? xml_out : xml_in,
- !info->inactive_only);
+ !info->inactive_only,
+ NULL, NULL);
  cleanup:
 VIR_FREE(xml_in);
 VIR_FREE(xml_out);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0134815..97838f3 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -42,7 +42,8 @@ testXML2XMLActive(const void *opaque)
 const struct testInfo *info = opaque;
 
 return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt,
-  info->inName, info->outActiveName, true);
+  info->inName, info->outActiveName, true,
+  NULL, NULL);
 }
 
 
@@ -52,7 +53,8 @@ testXML2XMLInactive(const void *opaque)
 const struct testInfo *info = opaque;
 
 return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName,
-  info->outInactiveName, false);
+  info->outInactiveName, false,
+  NULL, NULL);
 }
 
 
diff --git a/tests/testutils.c b/tests/testutils.c
index 10c26648..c282745 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -1097,7 +1097,9 @@ virDomainXMLOptionPtr 
virTestGenericDomainXMLConfInit(void)
 
 int
 testCompareDomXML2XMLFiles(virCapsPtr caps, virDomainXMLOptionPtr xmlopt,
-   const char *infile, const char *outfile, bool live)
+   const char *infile, const char *outfile, bool live,
+   testCompareDomXML2XMLPreFormatCallback cb,
+   const void *opaque)
 {
 char *actual = NULL;
 int ret = -1;
@@ -1115,6 +1117,9 @@ testCompareDomXML2XMLFiles(virCapsPtr caps, 
virDomainXMLOptionPtr xmlopt,
 goto fail;
 }
 
+if (cb && cb(def, opaque) < 0)
+goto fail;
+
 if (!(actual = virDomainDefFormat(def, format_flags)))
 goto fail;
 
diff --git a/tests/testutils.h b/tests/testutils.h
index b1d7397..df2b2a6 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -137,10 +137,14 @@ int virtTestMain(int argc,
 virCapsPtr virTestGenericCapsInit(void);
 virDomainXMLOptionPtr virTestGenericDomainXMLConfInit(void);
 
+typedef int (*testCompareDomXML2XMLPreFormatCallback)(virDomainDefPtr def,
+  const void *opaque);
 int testCompareDomXML2XMLFiles(virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
const char *inxml,
const char *outfile,
-   bool live);
+   bool live,
+   testCompareDomXML2XMLPreFormatCallback cb,
+   const void *opaque);
 
 #endif /* __VIT_TEST_UTILS_H__ */
-- 
2.5.0

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

[libvirt] [PATCH 1/2] qemu: aarch64: Don't add PCIe controller by default

[Cole Robinson]

This was discussed here:

https://www.redhat.com/archives/libvir-list/2015-December/msg00217.html

The summary is that apps/users are in a tough spot WRT aarch64 -M virt
and PCIe support: qemu has all the plumbing, but most distros don't
support it yet. Presently libvirt adds a PCIe controller to the XML for
new enough qemu, but this patch drops that behavior.

Upcoming patches will instead require users to manually specify a
pcie controller in the XML as a way of telling libvirt 'the OS supports
PCI', at which point libvirt can use it as a target for virtio-pci.
---
 src/qemu/qemu_domain.c |  4 
 .../qemuxml2argv-aarch64-virtio-pci-default.args   |  2 --
 tests/qemuxml2argvtest.c   |  6 ++
 .../qemuxml2xmlout-aarch64-virtio-pci-default.xml  | 10 --
 tests/qemuxml2xmltest.c|  2 ++
 5 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1df1b74..aea1ea5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1109,10 +1109,6 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 case VIR_ARCH_AARCH64:
 addDefaultUSB = false;
 addDefaultMemballoon = false;
-if (STREQ(def->os.machine, "virt") ||
-STRPREFIX(def->os.machine, "virt-")) {
-addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX);
-}
 break;
 
 case VIR_ARCH_PPC64:
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
index a49bc82..f879560 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
@@ -21,8 +21,6 @@ QEMU_AUDIO_DRV=none \
 -initrd /aarch64.initrd \
 -append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \
 -dtb /aarch64.dtb \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
--device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
 -device virtio-serial-device,id=virtio-serial0 \
 -usb \
 -drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index cfb46ef..b2f636e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1642,9 +1642,7 @@ mymain(void)
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM);
 
 /* Demonstrates the virtio-pci default... namely that there isn't any!
-   q35 style PCI controllers will be added if the binary supports it,
-   but virtio-mmio is always used unless PCI addresses are manually
-   specified. */
+   However this might change in the future... */
 DO_TEST("aarch64-virtio-pci-default",
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
 QEMU_CAPS_DEVICE_VIRTIO_MMIO,
@@ -1653,7 +1651,7 @@ mymain(void)
 QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
 /* Example of using virtio-pci with no explicit PCI controller
but with manual PCI addresses */
-DO_TEST("aarch64-virtio-pci-manual-addresses",
+DO_TEST_PARSE_ERROR("aarch64-virtio-pci-manual-addresses",
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
 QEMU_CAPS_DEVICE_VIRTIO_MMIO,
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
index 6f1c53b..db2119c 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
@@ -31,16 +31,6 @@
   
   
 
-
-
-  
-  
-
-
-  
-  
-  
-
 
   
 
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 581129c..cac401c 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -731,12 +731,14 @@ mymain(void)
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
 QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
 QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI);
+/*
 DO_TEST_FULL("aarch64-virtio-pci-manual-addresses", WHEN_ACTIVE,
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
 QEMU_CAPS_DEVICE_VIRTIO_MMIO,
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
 QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
 QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI);
+*/
 DO_TEST("aarch64-gic");
 DO_TEST("aarch64-gicv3");
 
-- 
2.5.0

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

[libvirt] [PATCH 1/9] tests: Run test-wrap-argv with REGENERATE_OUTPUT

[Cole Robinson]

To get properly wrapped output
---
 tests/testutils.c | 38 +++---
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index b587f83..10c26648 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -433,6 +433,32 @@ virtTestCaptureProgramOutput(const char *const argv[] 
ATTRIBUTE_UNUSED,
 }
 #endif /* !WIN32 */
 
+static int
+virTestRewrapFile(const char *filename)
+{
+int ret = -1;
+char *outbuf = NULL;
+char *script = NULL;
+virCommandPtr cmd = NULL;
+
+if (virAsprintf(, "%s/test-wrap-argv.pl", abs_srcdir) < 0)
+goto cleanup;
+
+cmd = virCommandNewArgList(script, filename, NULL);
+virCommandSetOutputBuffer(cmd, );
+if (virCommandRun(cmd, NULL) < 0)
+goto cleanup;
+
+if (virFileWriteStr(filename, outbuf, 0666) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(script);
+virCommandFree(cmd);
+VIR_FREE(outbuf);
+return ret;
+}
 
 /**
  * @param stream: output stream to write differences to
@@ -470,17 +496,15 @@ virtTestDifferenceFullInternal(FILE *stream,
 actualEnd = actual + (strlen(actual)-1);
 
 if (expectName && regenerate && (virTestGetRegenerate() > 0)) {
-char *regencontent;
-
-/* Try to properly indent qemu argv files */
-if (!(regencontent = virStringReplace(actual, " -", " \\\n-")))
+if (virFileWriteStr(expectName, actual, 0666) < 0) {
+virDispatchError(NULL);
 return -1;
+}
 
-if (virFileWriteStr(expectName, regencontent, 0666) < 0) {
-VIR_FREE(regencontent);
+if (virTestRewrapFile(expectName) < 0) {
+virDispatchError(NULL);
 return -1;
 }
-VIR_FREE(regencontent);
 }
 
 if (!virTestGetDebug())
-- 
2.5.0

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

[libvirt] [PATCH 0/9] tests: qemu: unconditionally enable QEMU_CAPS_DEVICE

[Cole Robinson]

Okay, my recent patch series' are kinda all over the place. I'm tableing
the src/qemu/ QEMU_CAPS_DEVICE bits for now. This is just the test
suite changes, which I need as a basis for other patches anyways.

This series does two main things: it conditionally enables QEMU_CAPS_DEVICE
in the test suite (since that's all we support nowadays), and it
unconditionally calls qemuDomainAssignAddresses for qemuxml2xml testing,
since that's the only realistic XML parsing scenario in the qemu driver.

This causes a lot of test output churn, so there's some other relevant
bits mixed in:

* An improvement to VIR_TEST_REGENERATE_OUTPUT
* Separating qemuargv2xml test output from qemuxml2argv, since the
  latter's output churn will massively break the former.
* More work to wire up qemuxml2xml test cases to allow passing in
  QEMU_CAPS_* lists, similar to qemuargv2xml, since
  qemuDomainAssignAddresses has many code paths dependent on QEMU_CAPS

* Have qemuxml2xml always use a separate output file. Martin commented
  on this here:

  https://www.redhat.com/archives/libvir-list/2016-January/msg01041.html

  Right now most of those tests expect the output to match the input, and
  have it baked  into the test framework. In order to maintain that paradigm,
  we would either have to update a large chunk of qemuxml2argv input data to
  contain static PCI addressing, or do a fine grained auditing of all the
  test cases to decide which ones we should be testing for different output
  vs ensure the same output.

  In my response to the above mail, I layed out my opinion that the
  qemuxml2xml tests are overloaded and it needs a much larger scale cleanup
  to be more practical. In the interim I think this change is in the right
  direction, since at least it's giving us XML output testing of qemu's
  specific address assignment functionality, which is always used in practice.
  Long term we should separate the generic XML testing from the qemu XML
  testing, and for the generic XML testing it likely makes sense to have
  many tests with matching input and output, so maybe we reintroduce
  that paradigm for genericxml2xml.


Note most of these patches have been on the list for over 2 weeks with
no review and rebasing them is conflict prone so I appreciate any timely
review. Thanks in advance


Cole Robinson (9):
  tests: Run test-wrap-argv with REGENERATE_OUTPUT
  tests: qemuxml2xml: Always use different output file
  tests: qemuargv2xml: separate from qemuxml2argv data
  tests: qemuxml2argv: remove some QEMU_CAPS_DEVICE problem cases
  tests: Unconditionally enable QEMU_CAPS_DEVICE
  tests: qemuxml2xml: Allow test cases to pass in qemuCaps
  tests: utils: Add PreFormat callback for CompareXML2XML helper
  tests: qemuxml2xml: assign device addresses
  tests: qemu: More aarch64 virtio and pci tests

 tests/Makefile.am  |   1 +
 tests/bhyvexml2xmltest.c   |   3 +-
 tests/domainschematest |   2 +-
 tests/genericxml2xmltest.c |   3 +-
 tests/lxcxml2xmltest.c |   3 +-
 .../qemuargv2xmldata/qemuargv2xml-boot-cdrom.args  |  22 +
 tests/qemuargv2xmldata/qemuargv2xml-boot-cdrom.xml |  31 ++
 .../qemuargv2xmldata/qemuargv2xml-boot-floppy.args |  23 +
 .../qemuargv2xmldata/qemuargv2xml-boot-floppy.xml  |  37 ++
 .../qemuargv2xml-boot-network.args |  22 +
 .../qemuargv2xmldata/qemuargv2xml-boot-network.xml |  30 ++
 .../qemuargv2xml-clock-localtime.args  |  23 +
 .../qemuargv2xml-clock-localtime.xml   |  30 ++
 .../qemuargv2xml-clock-utc.args}   |   5 +-
 tests/qemuargv2xmldata/qemuargv2xml-clock-utc.xml  |  30 ++
 .../qemuargv2xml-console-compat.args   |  22 +
 .../qemuargv2xml-console-compat.xml|  36 ++
 .../qemuargv2xml-disk-cdrom-empty.args |  23 +
 .../qemuargv2xml-disk-cdrom-empty.xml  |  36 ++
 .../qemuargv2xmldata/qemuargv2xml-disk-cdrom.args  |  23 +
 tests/qemuargv2xmldata/qemuargv2xml-disk-cdrom.xml |  37 ++
 .../qemuargv2xml-disk-drive-boot-cdrom.args|  23 +
 .../qemuargv2xml-disk-drive-boot-cdrom.xml |  37 ++
 .../qemuargv2xml-disk-drive-boot-disk.args |  23 +
 .../qemuargv2xml-disk-drive-boot-disk.xml  |  37 ++
 .../qemuargv2xml-disk-drive-cache-directsync.args  |  24 +
 .../qemuargv2xml-disk-drive-cache-directsync.xml   |  37 ++
 .../qemuargv2xml-disk-drive-cache-unsafe.args  |  23 +
 .../qemuargv2xml-disk-drive-cache-unsafe.xml   |  37 ++
 .../qemuargv2xml-disk-drive-cache-v2-none.args |  23 +
 .../qemuargv2xml-disk-drive-cache-v2-none.xml  |  37 ++
 .../qemuargv2xml-disk-drive-cache-v2-wb.args   |  24 +
 .../qemuargv2xml-disk-drive-cache-v2-wb.xml|  37 ++
 .../qemuargv2xml-disk-drive-cache-v2-wt.args   |  24 +
 .../qemuargv2xml-disk-drive-cache-v2-wt.xml|  37 ++
 ...uargv2xml-disk-drive-error-policy-enospace.args |  

[libvirt] [PATCH 9/9] tests: qemu: More aarch64 virtio and pci tests

[Cole Robinson]

Clarify the point of some of the test cases by renaming them. Add more
xml2xml tests.
---
 ...> qemuxml2argv-aarch64-virtio-pci-default.args} |  0
 ...=> qemuxml2argv-aarch64-virtio-pci-default.xml} |  0
 ...2argv-aarch64-virtio-pci-manual-addresses.args} |  0
 ...l2argv-aarch64-virtio-pci-manual-addresses.xml} |  0
 tests/qemuxml2argvtest.c   | 11 +++-
 .../qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml   |  6 +-
 .../qemuxml2xmlout-aarch64-virtio-pci-default.xml  | 69 ++
 ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 53 +
 tests/qemuxml2xmltest.c| 17 +-
 9 files changed, 152 insertions(+), 4 deletions(-)
 rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-mmio-default-pci.args => 
qemuxml2argv-aarch64-virtio-pci-default.args} (100%)
 rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-mmio-default-pci.xml => 
qemuxml2argv-aarch64-virtio-pci-default.xml} (100%)
 rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-virtio-pci.args => 
qemuxml2argv-aarch64-virtio-pci-manual-addresses.args} (100%)
 rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-virtio-pci.xml => 
qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml} (100%)
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.args 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
similarity index 100%
rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.args
rename to tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml
similarity index 100%
rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.xml
rename to tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.args 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args
similarity index 100%
rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.args
rename to 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml
similarity index 100%
rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.xml
rename to 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index fb08bb2..cfb46ef 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1640,13 +1640,20 @@ mymain(void)
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
 QEMU_CAPS_DEVICE_VIRTIO_MMIO,
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM);
-DO_TEST("aarch64-mmio-default-pci",
+
+/* Demonstrates the virtio-pci default... namely that there isn't any!
+   q35 style PCI controllers will be added if the binary supports it,
+   but virtio-mmio is always used unless PCI addresses are manually
+   specified. */
+DO_TEST("aarch64-virtio-pci-default",
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
 QEMU_CAPS_DEVICE_VIRTIO_MMIO,
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
 QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
 QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
-DO_TEST("aarch64-virtio-pci",
+/* Example of using virtio-pci with no explicit PCI controller
+   but with manual PCI addresses */
+DO_TEST("aarch64-virtio-pci-manual-addresses",
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
 QEMU_CAPS_DEVICE_VIRTIO_MMIO,
 QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml
index 4a31c8b..f79c6e7 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml
@@ -31,10 +31,13 @@
   
   
 
-
+
+  
+
 
   
   
+  
 
 
   
@@ -44,6 +47,7 @@
 
 
   /dev/random
+  
 
   
 
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
new file mode 100644
index 000..6f1c53b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
@@ -0,0 +1,69 @@
+
+  aarch64test
+  496d7ea8-9739-544b-4ebd-ef08be936e8b
+  1048576
+  

Re: [libvirt] [PATCH 00/17] qemu: Drop QEMU_CAPS_DEVICE part 2

[Cole Robinson]

On 01/22/2016 02:30 PM, Cole Robinson wrote:
> This is the second series dropping QEMU_CAPS_DEVICE. It handles
> qemu_command.c and all remaining uses of QEMU_CAPS_DEVICE.
> 
> Every qemu binary after v0.12.1 has the -device option, but many
> platforms cannot actually use it, because they are based around
> hardcoded machine models that cannot be extended. For these machine
> types, old style options work in _some_ scenarios, basically only
> situations where we are specifying host side config, where modern
> machines would use -chardev and -netdev.
> 
> Those code paths are already preserved for the only scenario that's
> actually been tested to work (qemu-system-arm -M vexpress and some
> other arm pieces), so they aren't going anywhere. But all other old
> style command line options can be dropped AFAICT. See individual
> patches for descriptions.
> 

Note, I'm pausing this series for now. I'm going to focus on getting the test
suite bits committed before I rebase and revive this.

thanks,
Cole

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

[libvirt] [PATCH 0/2] RFC: qemu: aarch64 virtio-pci work

[Cole Robinson]

These patches apply on top of the series I just posted:

  [PATCH 0/9] tests: qemu: unconditionally enable QEMU_CAPS_DEVICE
  https://www.redhat.com/archives/libvir-list/2016-January/msg01233.html

These specific changers were discussed here:

https://www.redhat.com/archives/libvir-list/2015-December/msg00217.html

We need a way for apps to tell libvirt 'use virtio-pci not virtio-mmio'
for aarch64 -M virt. We can't really make use this by default yet since
most OS don't actually support it. So we decided the best course of
interaction is to allow users to pass a manual pci controller, and libvirt
will interpret that to mean it's okay to use virtio-pci.

Patch #1 undoes the current behavior of unconditionally adding a q35 style
PCIe controller setup for mach-virt that supports it. In the future we
will probably re-enable something like this, but maybe not for a year or
so until the bits proliferate to distros.

Patch #2 tries to implement the manual pci controller bit. Unfortunately
it's not that easy get the ideal XML to work, so this isn't ready for
committing. See the commit message for more details


Cole Robinson (2):
  qemu: aarch64: Don't add PCIe controller by default
  HACK: qemu: aarch64: Use virtio-pci if user specifies PCI controller

 src/qemu/qemu_command.c| 34 
 src/qemu/qemu_domain.c |  4 --
 ...l2argv-aarch64-pci-manual-nocontroller-fail.xml | 43 
 .../qemuxml2argv-aarch64-virtio-pci-default.args   |  2 -
 ...l2argv-aarch64-virtio-pci-manual-addresses.args |  2 -
 ...ml2argv-aarch64-virtio-pci-manual-addresses.xml |  1 +
 ...2argv-aarch64-virtio-pci-manual-controller.args | 36 +
 ...l2argv-aarch64-virtio-pci-manual-controller.xml | 50 ++
 tests/qemuxml2argvtest.c   | 22 ++--
 .../qemuxml2xmlout-aarch64-virtio-pci-default.xml  | 10 
 ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 11 +---
 ...xmlout-aarch64-virtio-pci-manual-controller.xml | 60 ++
 tests/qemuxml2xmltest.c|  6 +++
 13 files changed, 239 insertions(+), 42 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-pci-manual-nocontroller-fail.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-controller.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-controller.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-controller.xml

-- 
2.5.0

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

[libvirt] [PATCH 4/9] tests: qemuxml2argv: remove some QEMU_CAPS_DEVICE problem cases

[Cole Robinson]

When we unconditionally enable QEMU_CAPS_DEVICE, these tests need
some massaging, so do it ahead of time to not mix it in with the
big test refresh.

- minimal-s390 is not a real world working config, so drop it
- disk-usb was testing for an old code path that will be removed.
  instead use it to test lack of USB disk support, and rename it
  to disk-usb-nosupport. Switch xml2xml to use disk-usb-device for
  input.
- cputune-numatune was needlessly using q35, switch it to an older
  machine type
---
 .../qemuxml2argv-cputune-numatune.args |  7 +--
 .../qemuxml2argv-cputune-numatune.xml  | 12 +---
 .../qemuxml2argv-disk-usb-nosupport.xml}   |  0
 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args  | 23 
 .../qemuxml2argv-minimal-s390.args | 21 ---
 .../qemuxml2argvdata/qemuxml2argv-minimal-s390.xml | 21 ---
 tests/qemuxml2argvtest.c   |  5 +-
 .../qemuxml2xmlout-cputune-numatune.xml| 14 +
 .../qemuxml2xmlout-disk-cdrom-empty.xml| 36 
 .../qemuxml2xmlout-disk-drive-boot-cdrom.xml   | 37 +
 .../qemuxml2xmlout-disk-drive-boot-disk.xml| 37 +
 .../qemuxml2xmlout-disk-drive-cache-directsync.xml | 37 +
 .../qemuxml2xmlout-disk-drive-cache-unsafe.xml | 37 +
 .../qemuxml2xmlout-disk-drive-cache-v2-none.xml| 37 +
 .../qemuxml2xmlout-disk-drive-cache-v2-wb.xml  | 37 +
 .../qemuxml2xmlout-disk-drive-cache-v2-wt.xml  | 37 +
 ...xml2xmlout-disk-drive-error-policy-enospace.xml | 37 +
 ...qemuxml2xmlout-disk-drive-error-policy-stop.xml | 37 +
 ...out-disk-drive-error-policy-wreport-rignore.xml | 37 +
 .../qemuxml2xmlout-disk-drive-network-gluster.xml  | 37 +
 .../qemuxml2xmlout-disk-drive-network-rbd-auth.xml | 42 ++
 ...uxml2xmlout-disk-drive-network-rbd-ceph-env.xml | 39 +
 .../qemuxml2xmlout-disk-drive-network-rbd-ipv6.xml | 40 ++
 .../qemuxml2xmlout-disk-drive-network-rbd.xml  | 64 ++
 .../qemuxml2xmlout-disk-drive-network-sheepdog.xml | 37 +
 .../qemuxml2xmlout-disk-usb-device.xml}|  6 +-
 .../qemuxml2xmloutdata/qemuxml2xmlout-migrate.xml  | 30 ++
 .../qemuxml2xmlout-misc-uuid.xml   | 33 +++
 .../qemuxml2xmlout-nographics-vga.xml  | 30 ++
 .../qemuxml2xmlout-qemu-ns-no-env.xml  | 34 
 .../qemuxml2xmlout-restore-v2.xml  | 30 ++
 .../qemuxml2xmloutdata/qemuxml2xmlout-watchdog.xml | 31 +++
 tests/qemuxml2xmltest.c|  2 +-
 33 files changed, 865 insertions(+), 99 deletions(-)
 rename tests/{qemuxml2xmloutdata/qemuxml2xmlout-disk-usb.xml => 
qemuxml2argvdata/qemuxml2argv-disk-usb-nosupport.xml} (100%)
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-cdrom-empty.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-boot-cdrom.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-boot-disk.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-cache-directsync.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-cache-unsafe.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-cache-v2-none.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-cache-v2-wb.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-cache-v2-wt.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-error-policy-enospace.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-error-policy-stop.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-error-policy-wreport-rignore.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-rbd-auth.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-rbd-ceph-env.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-rbd-ipv6.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-rbd.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-sheepdog.xml
 rename tests/{qemuxml2argvdata/qemuxml2argv-disk-usb.xml => 
qemuxml2xmloutdata/qemuxml2xmlout-disk-usb-device.xml} (90%)
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-migrate.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-misc-uuid.xml
 create mode 100644 

[libvirt] [PATCH 2/2] HACK: qemu: aarch64: Use virtio-pci if user specifies PCI controller

[Cole Robinson]

If a user manually specifies this XML snippet for aarch64 machvirt:

  

Libvirt will interpret this to mean that the OS supports virtio-pci,
and will allocate PCI addresses (instead of virtio-mmio) for virtio
devices.

This is a giant hack. Trying to improve it led me into the maze of PCI
address code and I gave up for now. Here are the issues:

* I'd prefer that to be model='pcie-root' which matches what
qemu-system-aarch64 -M virt actually provides by default... however
libvirt isn't happy with a single pcie-root specified by the user, it
will error with:

error: unsupported configuration: failed to create PCI bridge on bus 1: too 
many devices with fixed addresses

Instead this patch uses hacks to make pci-root use the pcie.0 bus for
aarch64, since that code path already works.

* It may even be nice to make specifying  map to
'give me the recommended PCI setup for this VM'... then we could adjust
what that default means in the future, maybe switching to something like
what q35 uses. This would make apps lives easier.

But presently that violates the XML schema, and libvirt can't handle lack
of model= anyways:

error: internal error: Invalid PCI controller model -1
---
 src/qemu/qemu_command.c| 34 
 ...l2argv-aarch64-pci-manual-nocontroller-fail.xml | 43 
 ...l2argv-aarch64-virtio-pci-manual-addresses.args |  2 -
 ...ml2argv-aarch64-virtio-pci-manual-addresses.xml |  1 +
 ...2argv-aarch64-virtio-pci-manual-controller.args | 36 +
 ...l2argv-aarch64-virtio-pci-manual-controller.xml | 50 ++
 tests/qemuxml2argvtest.c   | 20 ++--
 ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 11 +---
 ...xmlout-aarch64-virtio-pci-manual-controller.xml | 60 ++
 tests/qemuxml2xmltest.c|  8 ++-
 10 files changed, 239 insertions(+), 26 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-pci-manual-nocontroller-fail.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-controller.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-controller.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-controller.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8943270..4b0f070 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1064,7 +1064,10 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr 
domainDef,
  * (including the hardcoded pci-root controller on
  * multibus-capable qemus).
  */
-return virAsprintf(>info.alias, "pci.%d", controller->idx);
+if (domainDef->os.arch == VIR_ARCH_AARCH64)
+return virAsprintf(>info.alias, "pcie.%d", 
controller->idx);
+else
+return virAsprintf(>info.alias, "pci.%d", 
controller->idx);
 } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) {
 /* for any machine based on e.g. I440FX or G3Beige, the
  * first (and currently only) IDE controller is an integrated
@@ -1393,15 +1396,28 @@ static int
 qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
 {
-if (((def->os.arch == VIR_ARCH_ARMV7L) ||
-(def->os.arch == VIR_ARCH_AARCH64)) &&
-(STRPREFIX(def->os.machine, "vexpress-") ||
-STREQ(def->os.machine, "virt") ||
-STRPREFIX(def->os.machine, "virt-")) &&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) {
-qemuDomainPrimeVirtioDeviceAddresses(
-def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO);
+size_t i;
+
+if ((def->os.arch != VIR_ARCH_ARMV7L) &&
+(def->os.arch != VIR_ARCH_AARCH64))
+return 0;
+
+if (!(STRPREFIX(def->os.machine, "vexpress-") ||
+  STREQ(def->os.machine, "virt") ||
+  STRPREFIX(def->os.machine, "virt-")))
+return 0;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO))
+return 0;
+
+/* If there's a PCIe controller in the XML, we will use PCI virtio */
+for (i = 0; i < def->ncontrollers; i++) {
+if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI)
+return 0;
 }
+
+qemuDomainPrimeVirtioDeviceAddresses(
+def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO);
 return 0;
 }
 
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-pci-manual-nocontroller-fail.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-pci-manual-nocontroller-fail.xml
new file mode 100644
index 000..6a44f19
--- /dev/null
+++ 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-pci-manual-nocontroller-fail.xml
@@ -0,0 +1,43 @@
+
+  aarch64test
+  496d7ea8-9739-544b-4ebd-ef08be936e8b
+  1048576
+  1048576
+  1
+  
+hvm
+/aarch64.kernel
+/aarch64.initrd
+earlyprintk console=ttyAMA0,115200n8 rw 

[libvirt] [PATCH] pci: Use bool return type for some virPCIDeviceGet*() functions

[Andrea Bolognani]

The affected functions are:

  virPCIDeviceGetManaged()
  virPCIDeviceGetUnbindFromStub()
  virPCIDeviceGetRemoveSlot()
  virPCIDeviceGetReprobe()

Change their return type from unsigned int to bool: the corresponding
members in struct _virPCIDevice are defined as bool, and even the
corresponding virPCIDeviceSet*() functions take a bool value as input
so there's no point in these functions having unsigned int as return
type.

Suggested-by: John Ferlan 
---
 src/util/virpci.c | 8 
 src/util/virpci.h | 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 505c1f3..6554351 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1693,7 +1693,7 @@ void virPCIDeviceSetManaged(virPCIDevicePtr dev, bool 
managed)
 dev->managed = managed;
 }
 
-unsigned int
+bool
 virPCIDeviceGetManaged(virPCIDevicePtr dev)
 {
 return dev->managed;
@@ -1711,7 +1711,7 @@ virPCIDeviceGetStubDriver(virPCIDevicePtr dev)
 return dev->stubDriver;
 }
 
-unsigned int
+bool
 virPCIDeviceGetUnbindFromStub(virPCIDevicePtr dev)
 {
 return dev->unbind_from_stub;
@@ -1723,7 +1723,7 @@ virPCIDeviceSetUnbindFromStub(virPCIDevicePtr dev, bool 
unbind)
 dev->unbind_from_stub = unbind;
 }
 
-unsigned int
+bool
 virPCIDeviceGetRemoveSlot(virPCIDevicePtr dev)
 {
 return dev->remove_slot;
@@ -1735,7 +1735,7 @@ virPCIDeviceSetRemoveSlot(virPCIDevicePtr dev, bool 
remove_slot)
 dev->remove_slot = remove_slot;
 }
 
-unsigned int
+bool
 virPCIDeviceGetReprobe(virPCIDevicePtr dev)
 {
 return dev->reprobe;
diff --git a/src/util/virpci.h b/src/util/virpci.h
index d1ac942..5529b24 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -99,7 +99,7 @@ int virPCIDeviceReset(virPCIDevicePtr dev,
 
 void virPCIDeviceSetManaged(virPCIDevice *dev,
 bool managed);
-unsigned int virPCIDeviceGetManaged(virPCIDevice *dev);
+bool virPCIDeviceGetManaged(virPCIDevice *dev);
 void virPCIDeviceSetStubDriver(virPCIDevicePtr dev,
virPCIStubDriver driver);
 virPCIStubDriver virPCIDeviceGetStubDriver(virPCIDevicePtr dev);
@@ -110,13 +110,13 @@ int virPCIDeviceSetUsedBy(virPCIDevice *dev,
 void virPCIDeviceGetUsedBy(virPCIDevice *dev,
const char **drv_name,
const char **dom_name);
-unsigned int virPCIDeviceGetUnbindFromStub(virPCIDevicePtr dev);
+bool virPCIDeviceGetUnbindFromStub(virPCIDevicePtr dev);
 void  virPCIDeviceSetUnbindFromStub(virPCIDevice *dev,
 bool unbind);
-unsigned int virPCIDeviceGetRemoveSlot(virPCIDevicePtr dev);
+bool virPCIDeviceGetRemoveSlot(virPCIDevicePtr dev);
 void virPCIDeviceSetRemoveSlot(virPCIDevice *dev,
bool remove_slot);
-unsigned int virPCIDeviceGetReprobe(virPCIDevicePtr dev);
+bool virPCIDeviceGetReprobe(virPCIDevicePtr dev);
 void virPCIDeviceSetReprobe(virPCIDevice *dev,
 bool reprobe);
 void virPCIDeviceReattachInit(virPCIDevice *dev);
-- 
2.5.0

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

[libvirt] [PATCH] qemu: return -1 on error paths in qemuDomainSaveImageStartVM

[Nikolay Shirokovskiy]

The error paths after fix suppose ret == -1 as at the
beginning of the function.

The better fix would be not to touch 'ret' at all until
the end of the function where it is set to 0. Thus we'd
better introduce some 'rc' variable to hold return
values of called functions.
---
 src/qemu/qemu_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 351e529..dfbf846 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6790,6 +6790,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
 virDomainAuditStart(vm, "restored", false);
 goto cleanup;
 }
+ret = -1;
 
 event = virDomainEventLifecycleNewFromObj(vm,
  VIR_DOMAIN_EVENT_STARTED,
-- 
1.8.3.1

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

Re: [libvirt] Call for mentors and project ideas for Google Summer of Code 2016

[Stefan Hajnoczi]

On Wed, Jan 27, 2016 at 12:06:22PM +0100, Michal Privoznik wrote:
> On 27.01.2016 11:56, Stefan Hajnoczi wrote:
> > On Tue, Jan 26, 2016 at 11:13:16AM +0100, Michal Privoznik wrote:
> >> On 25.01.2016 18:28, Stefan Hajnoczi wrote:
> >>> The QEMU wiki page for Google Summer of Code 2016 is now available here:
> >>>
> >>> http://qemu-project.org/Google_Summer_of_Code_2016
> >>>
> >>> QEMU will apply for Google Summer of Code 2016 (https://g.co/gsoc/).
> >>> If QEMU is accepted there will be funding for students to work on
> >>> 12-week full-time open source projects remotely from May to August
> >>> 2016.  QEMU provides a mentor for each student who gives advice and
> >>> evaluates their progress.
> >>>
> >>> If you have a project idea, especially if you are a regular
> >>> contributor to QEMU and are willing to mentor this summer, please go
> >>> to this wiki page and fill out the project idea template:
> >>>
> >>> http://qemu-project.org/Google_Summer_of_Code_2016
> >>>
> >>> The project ideas list is part of the application so that QEMU can
> >>> participate in GSoC.  It's useful to have your project ideas on the
> >>> wiki by February 8th 2016.
> >>>
> >>> If you have any questions about project ideas or QEMU applying to
> >>> GSoC, please reply to this thread.
> >>
> >> Hey Stefan,
> >>
> >> so as we spoke earlier in person, I think it's time for libvirt to try
> >> and apply as a separate organization.
> > 
> > Great!
> > 
> >> I went ahead and created similar GSoC ideas page for libvirt:
> >>
> >> http://wiki.libvirt.org/page/Google_Summer_of_Code_2016
> >>
> >> My question is, is qemu willing to back libvirt in case we don't get
> >> selected and if so, should we duplicate the idea list into qemu wiki too?
> > 
> > I'm worried that this would look like orgs trying to game the system.
> 
> You're right. Maybe we should not do it after all.
> 
> > 
> > Are there project ideas that involve both QEMU and libvirt changes?  For
> > those I think it's fair to have them in both org's lists.  Anything that
> > is purely QEMU or libvirt shouldn't be duplicated.
> > 
> 
> So far none. But I have adjusted both our wikis to note that.

By the way, you are welcome to put the QEMU org as a reference on the
application form for libvirt.  It boosts your chance slightly as a new
org to have another org vouch for you.

Stefan


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

Re: [libvirt] [PATCH] qemu: return -1 on error paths in qemuDomainSaveImageStartVM

[Jiri Denemark]

On Thu, Jan 28, 2016 at 15:49:48 +0300, Nikolay Shirokovskiy wrote:
> The error paths after fix suppose ret == -1 as at the
> beginning of the function.
> 
> The better fix would be not to touch 'ret' at all until
> the end of the function where it is set to 0. Thus we'd
> better introduce some 'rc' variable to hold return
> values of called functions.

Exactly, it would be much better the way you describe, so why didn't you
do it that way? :-)

NACK to this patch.

Jirka

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

[libvirt] [PATCH] rbd: Open in Read-Only mode when refreshing a volume

[Wido den Hollander]

By opening a RBD volume in Read-Only we do not register a
watcher on the header object inside the Ceph cluster.

Refreshing a volume only calls rbd_stat() which is a operation
which does not write to a RBD image.

This allows us to use a cephx user which has no write
permissions if we would want to use the libvirt storage pool
for informational purposes only.

It also saves us a write into the Ceph cluster which should
speed up refreshing a RBD pool.

rbd_open_read_only() is available in all librbd versions which
also support rbd_open().

Signed-off-by: Wido den Hollander 
---
 src/storage/storage_backend_rbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 8c7a80d..3ab7912 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -283,7 +283,7 @@ static int 
volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
 int r = 0;
 rbd_image_t image = NULL;
 
-r = rbd_open(ptr->ioctx, vol->name, , NULL);
+r = rbd_open_read_only(ptr->ioctx, vol->name, , NULL);
 if (r < 0) {
 ret = -r;
 virReportSystemError(-r, _("failed to open the RBD image '%s'"),
-- 
1.9.1

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

Re: [libvirt] [PATCH 4/4] logical: Adjust regex for devices

[Pavel Hrdina]

On Wed, Jan 27, 2016 at 03:26:12PM -0500, John Ferlan wrote:
> ...
> >>
> >> However, after some more "investigation" of the environment - I think
> >> perhaps there's still a couple of loose ends. Keeping the 'segtype'
> >> field may be necessary/useful... Details follow if interested ;-)
> > 
> > Yes, you're right and I did a bad job during review.  The segtype is 
> > required
> > and we should always consider it, there is like 20 segtypes right now and 
> > some
> > of them could also use 'stripes'.
> > 
> 
> The 'segtype' is currently only required because commit id '82c1740a'
> added 'segtype' and 'stripes' to resolve a bz (or more than one
> depending on how far you chase) by using 'segtype' to determine whether
> more than one existed. It also did quite a few things in one step which
> by today's review standards is a bad thing ;-).
> 
> However, that only worked for "striped" lv's. If there was a "mirror"
> lv, then while the mirror could be found, the vol-dumpxml output is
> wrong because it only parsed 1 extent (incorrectly at that):

That leads me to conclusion, that we should parse only the segtypes that we know
how to parse.  The rest should be ignored.

> 
>   
> 
>   
> 
>   
> 
> instead of something like (if using 'stripes' to get nsegments):
> 
>   
> 
>   
> 
> 
>   
> 
>   
> 
> Linking 'nsegments' to 'striped' lv's is a "limitation".
> 
> Anyway, dropping 'segtype' wasn't necessarily bad unless you needed
> "more information", such as perhaps the lv thin pool source when
> nsegments == 0. This becomes obvious once the change to use "(\\S*)"
> instead of "(\\S+)" hits. Now we can "see" thin lv's in a volume group.
> Not sure what else becomes visible, though.  The problem is you then get:

But we need more information and we propagate those information to user via our
API and that's why I think that we should parse only those volume which we know
how to parse to pass correct values to user.

Pavel

> 
>  
>  
> 
> But that's fixable... The interesting part about  is that it's
> an output only (virStorageVolDefParseXML doesn't read it). So, by adding
> a new parse field 'pool_lv', we can then check 'segtype' to be "thin"
> and if so, store the new field in a new vol->source.thin_pool field.
> 
> Still cannot create a thin lv pool/volume without using the lvcreate
> command. Nor can one create a mirror or stripe lv using libvirt's
> vol-create command. One just cannot "find" a thin lv right now...
> 
> John
> 

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

Re: [libvirt] [PATCH v2 8/9] pci: Phase out virPCIDeviceReattachInit()

[Andrea Bolognani]

On Tue, 2016-01-26 at 18:59 -0500, John Ferlan wrote:
> 
> On 01/25/2016 11:21 AM, Andrea Bolognani wrote:
> > The name is confusing, and the only use left is in a test case.
> > ---
> >  src/libvirt_private.syms | 1 -
> >  src/util/virpci.c| 8 
> >  src/util/virpci.h| 1 -
> >  tests/virpcitest.c   | 5 -
> >  4 files changed, 4 insertions(+), 11 deletions(-)
> > 
> 
> Seems reasonable - ACK

This has been pushed.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH v2 9/9] pci: Add debug messages when unbinding from stub driver

[Andrea Bolognani]

On Tue, 2016-01-26 at 19:00 -0500, John Ferlan wrote:
>  
> > -VIR_DEBUG("Found stub driver %s", driver);
> > +VIR_DEBUG("Found stub driver %s for PCI device %s", driver, dev->name);
> > +VIR_DEBUG("Unbinding PCI device %s", dev->name);
> 
> Redundant - How about "Found stub driver %s to unbind PCI device %s" or
> "Unbinding PCI device %s for stub driver %s" (don't forget to change
> order of args ;-))
> 
> IOW: No need to have two messages.
> 
> Hope they help some day!
> 
> ACK -

I ended up using "Unbinding PCI device %s from stub driver %s" after all.

Pushed.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH] gendispatch: Don't output spaces on empty line

[Jiri Denemark]

On Thu, Jan 28, 2016 at 15:23:56 +0100, Michal Privoznik wrote:
> In our generator for some code we put empty lines in the output
> to separate some blocks of code. However, in some cases we put
> couple of spaces on the empty line too. It's not bug, it just
> isn't nice.

Too many "some" :-)

> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/rpc/gendispatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
> index 5cfc512..3740130 100755
> --- a/src/rpc/gendispatch.pl
> +++ b/src/rpc/gendispatch.pl
> @@ -677,7 +677,7 @@ elsif ($mode eq "server") {
>  push(@prepare_ret_list,
>   "if (VIR_ALLOC($2_p) < 0)\n" .

Looks like the spaces should have been here, in front of "if".

>   "goto cleanup;\n" .
> - "\n" .
> + "\n" .
>   "if (VIR_STRDUP(*$2_p, $2) < 0)\n".
>   "goto cleanup;\n");

ACK

Jirka

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

Re: [libvirt] [PATCH v6 0/7] Global domain cpu.cfs_period_us and cpu.cfs_quota_us setup

[Alexander Burluka]

Ping

On 01/27/2016 07:17 PM, Alexander Burluka wrote:

This patchset implements an ability to specify values for domain top level
cpu.cfs_period_us and cpu.cfs_quota_us cgroups. These parameters are opt-in
and named "global_period" and "global_quota".

Introduction of these settings gives management applications further
choice of controlling CPU usage.

Changes in v2: add XML validation test
Changes in v3: remove unneccessary cgroup copying
Changes in v4: fix little rebase error
Changes in v5: rebase to version 1.3.1
Changes in v6: remove unnecessary check

Alexander Burluka (7):
   Add global period definitions
   Add global quota parameter necessary definitions
   Add error checking on global quota and period
   Add global_period and global_quota XML validation test
   Rename qemuSetupCgroupVcpuBW to qemuSetupBandwidthCgroup
   Implement qemuSetupGlobalCpuCgroup
   Implement handling of per-domain bandwidth settings

  docs/schemas/domaincommon.rng   |  10 +++
  include/libvirt/libvirt-domain.h|  32 
  src/conf/domain_conf.c  |  37 +
  src/conf/domain_conf.h  |   2 +
  src/qemu/qemu_cgroup.c  |  68 ++--
  src/qemu/qemu_cgroup.h  |   7 +-
  src/qemu/qemu_command.c |   3 +-
  src/qemu/qemu_driver.c  | 102 ++--
  src/qemu/qemu_process.c |   4 +
  tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |   2 +
  10 files changed, 251 insertions(+), 16 deletions(-)



--
Regards,
Alexander Burluka

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

[libvirt] [libvirt-php] add flags support to virStorageVolCreateXML

[Vasiliy Tolstov]

Signed-off-by: Vasiliy Tolstov 
---
 src/libvirt-php.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index f3b3f9f81e6d..a0c960957edc 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -1238,6 +1238,8 @@ PHP_MINIT_FUNCTION(libvirt)
 REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_ALLOCATE",1, 
CONST_CS | CONST_PERSISTENT);
 REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_DELTA",   2, 
CONST_CS | CONST_PERSISTENT);
 REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_SHRINK",  4, 
CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA", 
VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_CREATE_REFLINK", 
VIR_STORAGE_VOL_CREATE_REFLINK, CONST_CS | CONST_PERSISTENT);
 
 /* Domain vCPU flags */
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_CONFIG",
VIR_DOMAIN_VCPU_CONFIG, CONST_CS | CONST_PERSISTENT);
@@ -7105,6 +7107,7 @@ PHP_FUNCTION(libvirt_storagevolume_get_xml_desc)
  * Description: Function is used to create the new storage pool and return 
the handle to new storage pool
  * Arguments:   @res [resource]: libvirt storagepool resource
  *  @xml [string]: XML string to create the storage volume in 
the storage pool
+ *  @flags [int]: virStorageVolCreateXML flags
  * Returns: libvirt storagevolume resource
  */
 PHP_FUNCTION(libvirt_storagevolume_create_xml)
@@ -7114,11 +7117,12 @@ PHP_FUNCTION(libvirt_storagevolume_create_xml)
 zval *zpool;
 virStorageVolPtr volume=NULL;
 char *xml;
+long flags = 0;
 int xml_len;
 
-GET_STORAGEPOOL_FROM_ARGS("rs",,,_len);
+GET_STORAGEPOOL_FROM_ARGS("rs|l",,,_len, );
 
-volume=virStorageVolCreateXML(pool->pool,xml,0);
+volume=virStorageVolCreateXML(pool->pool, xml, flags);
 DPRINTF("%s: virStorageVolCreateXML(%p, , 0) returned %p\n", PHPFUNC, 
pool->pool, volume);
 if (volume==NULL) RETURN_FALSE;
 
-- 
2.7.0

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

Re: [libvirt] [PATCH 10/34] qemu: Don't use priv->ncpus to iterate cgroup setting

[Peter Krempa]

On Sat, Jan 16, 2016 at 10:22:13 -0500, John Ferlan wrote:
> 
> 
> On 01/14/2016 11:26 AM, Peter Krempa wrote:
> > Iterate over all cpus skipping inactive ones.
> > ---
> >  src/qemu/qemu_driver.c | 15 ---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> 
> Patch 7 introduces virDomainDefGetVcpumap (or GetOnlineVcpuMap) - why
> not use that instead and then iterate the set bits.  This works, but the
> less places that check for vcpu->online perhaps the better. Perhaps also
> a way to reduce the decision points of using ->maxvcpus or the
> virDomainDefGetVcpusMax call...

That would end up in two cases:

1) The bitmap would be recalculated before every use.
This increases complexity twofold since the function iterates the list
once to assemble the bitmap and then you iterate the bitmap to get the
objects.

2) The bitmap would need to be stored persistently
This again introduces two different places where data has to be stored.
It either then will require us to keep it in sync the two places or
remove one and the def->vcpus and def->vcpumap would need to be used in
parallel always.

I don't like either of those since the target was to remove two places
storing data about one object.

Peter


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

[libvirt] [PATCH] gendispatch: Don't output spaces on empty line

[Michal Privoznik]

In our generator for some code we put empty lines in the output
to separate some blocks of code. However, in some cases we put
couple of spaces on the empty line too. It's not bug, it just
isn't nice.

Signed-off-by: Michal Privoznik 
---
 src/rpc/gendispatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 5cfc512..3740130 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -677,7 +677,7 @@ elsif ($mode eq "server") {
 push(@prepare_ret_list,
  "if (VIR_ALLOC($2_p) < 0)\n" .
  "goto cleanup;\n" .
- "\n" .
+ "\n" .
  "if (VIR_STRDUP(*$2_p, $2) < 0)\n".
  "goto cleanup;\n");
 
-- 
2.4.10

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

Re: [libvirt] [PATCH v2 1/9] hostdev: Add virHostdevOnlyReattachPCIDevice()

[John Ferlan]

On 01/27/2016 12:26 PM, Andrea Bolognani wrote:
> On Tue, 2016-01-26 at 18:55 -0500, John Ferlan wrote:
>>  
>> w/r/t: your [0/7] from initial series...
>>  
>> As much as you don't want to keep living Groundhog Day - resolution of
>> bugs like this are job security :-)...
> 
> Groundhog Day is in less than a week, by the way! :)
> 
>> w/r/t Suggestions for deamon restart issues... Seems like we need a way
>> to save/restore the hostdev_mgr active/inactive lists using XML/JSON
>> similar to how domains, storage, etc. handle it. Guess I just assumed
>> that was already there ;-) since /var/run/libvirt/hostdevmgr exists. It
>> seems that network stuff can be restored - virHostdevNetConfigRestore.
>>  
>> Do you really think this series should be "held up" waiting to create
>> some sort of status tracking?
> 
> I will look into your suggestion. I believe such save / restore
> functionality has to be in place by the time this series is merged if
> we don't want to break everything on daemon restart.
> 

I assume that restart is already broken... This series does fix some
issues in the "normal" flow though. Perhaps a chicken and egg type
problem. If you fix restart first, what of this series would be
beneficial to ensure restart doesn't have issues...

>> On 01/25/2016 11:20 AM, Andrea Bolognani wrote:
>>> This function replaces virHostdevReattachPCIDevice() and, unlike it,
>>> does not perform list manipulation, leaving it to the calling function.
>>>  
>>> This means virHostdevReAttachPCIDevices() had to be updated to cope
>>> with the updated requirements.
>>> ---
>>>   src/util/virhostdev.c | 136 
>>> +-
>>>   1 file changed, 90 insertions(+), 46 deletions(-)
>>  
>> Since I reviewed them all... I think the comment changes from 7/9 should
>> just be inlined here and patch 4 instead of a separate patch
> 
> Will do - it was that way in v1 as well.
> 

Right - I started looking at v2

>>> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
>>> index f31ad41..66629b4 100644
>>> --- a/src/util/virhostdev.c
>>> +++ b/src/util/virhostdev.c
>>> @@ -526,6 +526,74 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr 
>>> hostdev,
>>>   return ret;
>>>   }
>>>   
>>> +/**
>>> + * virHostdevOnlyReattachPCIDevice:
>>  
>> Why not just reuse the function name you deleted? IOW: Is there a reason
>> for "Only"? (not that I'm one that can complain about naming functions,
>> but this just seems strange).
> 
> It's an attempt to make it stand out a bit from
> 
>   virHostdevPCINodeDeviceReAttach()
>   virHostdevReAttachPCIDevices()
> 
> in the same file. Mostly the latter.
> 
> The reasoning behind "Only" is that the function performs "Only" the job
> of reattaching the device to the host, with the error checking,
> bookkeeping and additional steps left to the caller.
> 
> Which is, strictly speaking, not true :)
> 
> Maybe something like virHostdevReattachPCIDeviceCommon(), to express the
> fact that this basically contains as much functionality as it was
> possible to split off to a reusable routine?
> 

virHostdevReattachPCIDeviceVeryCarefully   :-)

But since it's in the comment of the code:

virHostdevReattachPCIDeviceToHost

>>> + * @mgr: hostdev manager
>>> + * @pci: PCI device to be reattached
>>  
>> Interesting ... In 2 instances, this will be a pointer to the "copy"
>> element, while in the third instance this will be the "actual" on
>> inactive list element.  For a copy element, we'd *have* to search
>> inactive; however, for an 'actual' we don't "need" to.
> 
> Good point.
> 
> I will try to find a solution that
> 
>   1. avoids searching the list twice
>   2. avoids duplicating code
>   3. respects the Principle of Least Surprise
> 
> I can't guarantee I'll be able to :)
> 

I kept losing focus on when something was on the inactive list or not.
Then of course trying to reconcile 'pci' and 'dev' variable name usage.

>>> + * @skipUnmanaged: whether to skip unmanaged devices
>>> + *
>>> + * Reattach a PCI device to the host.
>>> + *
>>> + * This function only performs the base reattach steps that are required
>>> + * regardless of whether the device is being detached from a domain or
>>> + * had been simply detached from the host earlier.
>>> + *
>>> + * @pci must have already been marked as inactive, and the PCI related
>>> + * parts of @mgr (inactivePCIHostdevs, activePCIHostdevs) must have been
>>> + * locked beforehand using virObjectLock().
>>> + *
>>> + * Returns: 0 on success, <0 on failure
>>> + */
>>> +static int
>>> +virHostdevOnlyReattachPCIDevice(virHostdevManagerPtr mgr,
>>> +virPCIDevicePtr pci,
>>> +bool skipUnmanaged)
>>> +{
>>> +virPCIDevicePtr actual;
>>> +int ret = -1;
>>> +
>>> +/* Retrieve the actual device from the inactive list */
>>> +if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci))) {
>>> +VIR_DEBUG("PCI device %s is not 

Re: [libvirt] [PATCH] qemu: return -1 on error paths in qemuDomainSaveImageStartVM

[Nikolay Shirokovskiy]

On 28.01.2016 16:27, Jiri Denemark wrote:
> On Thu, Jan 28, 2016 at 15:49:48 +0300, Nikolay Shirokovskiy wrote:
>> The error paths after fix suppose ret == -1 as at the
>> beginning of the function.
>>
>> The better fix would be not to touch 'ret' at all until
>> the end of the function where it is set to 0. Thus we'd
>> better introduce some 'rc' variable to hold return
>> values of called functions.
> 
> Exactly, it would be much better the way you describe, so why didn't you
> do it that way? :-)
Ok)
> 
> NACK to this patch.
> 
> Jirka
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] [PATCH v2 3/9] hostdev: Use common reattach code in virHostdevPCINodeDeviceReAttach()

[John Ferlan]

On 01/27/2016 12:39 PM, Andrea Bolognani wrote:
> On Tue, 2016-01-26 at 18:56 -0500, John Ferlan wrote:
>>> +/**
>>> + * virHostdevPCINodeDeviceReAttach:
>>  
>> ^^ Oy ReAttach vs. Reattach is an eye test ;-)
> 
> Maybe we should standardize on either one or the other? I personally
> consider "ReAttach" to be quite an eyesore, but then again it's all
> over the public API so it's not going anywhere...
> 

Sadly, I think we're stuck with that CaMel case because it's a driver
function...

>>> + * @hostdev_mgr: hostdev manager
>>> + * @pci: PCI device
>>  
>> Perhaps better to indicate a "new"ly generated PCI device that does not
>> track the internal reattach states and other state information such as
>> the stub driver.
>>  
>> IOW: this is not a copy of an [in]activePCIHostdevs element
> 
> Great idea! Maybe even use a different name for the parameter, based
> on whether the virPCIDevicePtr is going to be used for something other
> than looking up the actual device?

This is I believe where I went back to patch 1 and started thinking
about what is passed in 'pci'...  Anything to help make things more
obvious could be beneficial, especially considering what this code ends
up doing...

John
> 
> Cheers.
> 
> -- 
> Andrea Bolognani
> Software Engineer - Virtualization Team
> 

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

Re: [libvirt] [PATCH] pci: Use bool return type for some virPCIDeviceGet*() functions

[Andrea Bolognani]

On Thu, 2016-01-28 at 10:35 -0500, John Ferlan wrote:
> 
> On 01/28/2016 03:21 AM, Andrea Bolognani wrote:
> > The affected functions are:
> > 
> >   virPCIDeviceGetManaged()
> >   virPCIDeviceGetUnbindFromStub()
> >   virPCIDeviceGetRemoveSlot()
> >   virPCIDeviceGetReprobe()
> > 
> > Change their return type from unsigned int to bool: the corresponding
> > members in struct _virPCIDevice are defined as bool, and even the
> > corresponding virPCIDeviceSet*() functions take a bool value as input
> > so there's no point in these functions having unsigned int as return
> > type.
> > 
> > Suggested-by: John Ferlan 
> > ---
> >  src/util/virpci.c | 8 
> >  src/util/virpci.h | 8 
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> > 
> 
> ACK

Pushed, thanks.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH 1/8] perf: add new public APIs for perf event

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:31PM +0800, Qiaowei Ren wrote:
> API agreed on in
> https://www.redhat.com/archives/libvir-list/2015-October/msg00872.html
> 
> * include/libvirt/libvirt-domain.h (virDomainGetPerfEvents,
> virDomainSetPerfEvents): New declarations.
> * src/libvirt_public.syms: Export new symbols.
> * src/driver-hypervisor.h (virDrvDomainGetPerfEvents,
> virDrvDomainSetPerfEvents): New typedefs.
> * src/libvirt-domain.c: Implement virDomainGetPerfEvents and
> virDomainSetPerfEvents.
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  include/libvirt/libvirt-domain.h | 18 
>  src/driver-hypervisor.h  | 12 ++
>  src/libvirt-domain.c | 93 
> 
>  src/libvirt_public.syms  |  6 +++
>  4 files changed, 129 insertions(+)

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

[Daniel P. Berrange]

On Thu, Dec 10, 2015 at 08:34:33PM +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 |   1 +
>  src/Makefile.am |   1 +
>  src/libvirt_private.syms|  12 ++
>  src/util/virerror.c |   1 +
>  src/util/virperf.c  | 303 
> 
>  src/util/virperf.h  |  63 +
>  6 files changed, 381 insertions(+)
>  create mode 100644 src/util/virperf.c
>  create mode 100644 src/util/virperf.h

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