[libvirt] [PATCH v4] Use-correct-pci-addresses-during-interface-detach

[Nitesh Konkar]

The virsh attach virsh detach interface command fails  when both live and config
are set and when the  interface gets attached to different pci slots
on live and config xml respectively.

When we attach an interface with both --live and --config,
the first time they get the same PCI slots, but the second time
onwards it differs and hence the virsh detach-interface --live
--config command fails. This patch makes sure that when both
--live --config are set , qemuDomainDetachDeviceFlags is called
twice, once with config xml and once with live xml.
---
Change log:
v4:
* Code unchanged, updated with commit message,change log 
  and steps to reproduce the issue..

v3:
* Created function vshDomainDetachInterface and called 
  it once/twice from cmdDetachInterface depending on 
  number of flags set (live/config/both). Passed the 
  correct xml(live/persistent) to it. 

v2:
* Changes only in cmdDetachInterface to pass the right domain xml
  depending on the flag set (live/config/both).

v1:
* Changes only in qemuDomainDetachDeviceFlags to set the right value 
  in dev and dev_copy.

Steps to see the issue:
virsh attach-interface --domain DomainName --type network --source default 
--mac 52:54:00:4b:76:5f --live --config
virsh detach-interface --domain DomainName --type network --mac 
52:54:00:4b:76:5f --live --config
virsh attach-interface --domain DomainName --type network --source default 
--mac 52:54:00:4b:76:5f --live --config
virsh detach-interface --domain DomainName --type network --mac 
52:54:00:4b:76:5f --live --config

 tools/virsh-domain.c | 104 +++
 1 file changed, 64 insertions(+), 40 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 62acecb..a6abaf5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10801,40 +10801,21 @@ static const vshCmdOptDef opts_detach_interface[] = {
 {.name = NULL}
 };
 
-static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+static bool 
+vshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, 
vshControl *ctl, const vshCmd *cmd)
 {
-virDomainPtr dom = NULL;
 xmlDocPtr xml = NULL;
 xmlXPathObjectPtr obj = NULL;
 xmlXPathContextPtr ctxt = NULL;
 xmlNodePtr cur = NULL, matchNode = NULL;
-char *detach_xml = NULL;
 const char *mac = NULL, *type = NULL;
-char *doc = NULL;
+char *detach_xml = NULL;
+bool current = vshCommandOptBool(cmd, "current");
 char buf[64];
 int diff_mac;
 size_t i;
 int ret;
 bool functionReturn = false;
-unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
-bool current = vshCommandOptBool(cmd, "current");
-bool config = vshCommandOptBool(cmd, "config");
-bool live = vshCommandOptBool(cmd, "live");
-bool persistent = vshCommandOptBool(cmd, "persistent");
-
-VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
-
-VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
-VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
-
-if (config || persistent)
-flags |= VIR_DOMAIN_AFFECT_CONFIG;
-if (live)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-return false;
 
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 goto cleanup;
@@ -10842,15 +10823,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "mac", ) < 0)
 goto cleanup;
 
-if (persistent &&
-virDomainIsActive(dom) == 1)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-if (flags & VIR_DOMAIN_AFFECT_CONFIG)
-doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
-else
-doc = virDomainGetXMLDesc(dom, 0);
-
 if (!doc)
 goto cleanup;
 
@@ -10918,23 +10890,75 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 else
 ret = virDomainDetachDevice(dom, detach_xml);
 
-if (ret != 0) {
-vshError(ctl, "%s", _("Failed to detach interface"));
-} else {
-vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+if (ret == 0) 
 functionReturn = true;
-}
 
  cleanup:
-VIR_FREE(doc);
 VIR_FREE(detach_xml);
-virDomainFree(dom);
+xmlFreeDoc(xml);
 xmlXPathFreeObject(obj);
 xmlXPathFreeContext(ctxt);
-xmlFreeDoc(xml);
 return functionReturn;
 }
 
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+char *doc_live = NULL, *doc_config = NULL;
+bool current = vshCommandOptBool(cmd, "current");
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+bool persistent = vshCommandOptBool(cmd, "persistent");
+bool functionReturn = false;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config 

Re: [libvirt] [PATCH] Report error when both live and offline flags are used for migration.

[Nitesh Konkar]

Hello All,

Any comments on this small patch?

Warm Regards,
Nitesh Konkar.

On Thu, Mar 3, 2016 at 4:38 PM, Nitesh Konkar <
niteshkonkar.libv...@gmail.com> wrote:

> ---
>  src/libvirt-domain.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 9491845..dc11945 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain,
>   error);
>
> +if (flags & VIR_MIGRATE_OFFLINE) {
> +if (flags & VIR_MIGRATE_LIVE) {
> +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +   _("Live and offline migration flags are "
> + "mutually exclusive"));
> +goto error;
> +}
> +}
> +
>  if (flags & VIR_MIGRATE_OFFLINE) {
>  if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>VIR_DRV_FEATURE_MIGRATION_OFFLINE))
> {
>  virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> @@ -3841,6 +3850,15 @@ virDomainMigrate2(virDomainPtr domain,
>   error);
>
> +if (flags & VIR_MIGRATE_OFFLINE) {
> +if (flags & VIR_MIGRATE_LIVE) {
> +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +   _("Live and offline migration flags are "
> + "mutually exclusive"));
> +goto error;
> +}
> +}
> +
>  if (flags & VIR_MIGRATE_OFFLINE) {
>  if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>VIR_DRV_FEATURE_MIGRATION_OFFLINE))
> {
>  virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> @@ -4014,6 +4032,15 @@ virDomainMigrate3(virDomainPtr domain,
>   VIR_MIGRATE_NON_SHARED_INC,
>   error);
>
> +if (flags & VIR_MIGRATE_OFFLINE) {
> +if (flags & VIR_MIGRATE_LIVE) {
> +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +   _("Live and offline migration flags are "
> + "mutually exclusive"));
> +goto error;
> +}
> +}
> +
>  if (flags & VIR_MIGRATE_PEER2PEER) {
>  virReportInvalidArg(flags, "%s",
>  _("use virDomainMigrateToURI3 for
> peer-to-peer "
> --
> 1.8.3.1
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V2 0/4] Extend to a tristate

[Jim Fehlig]

On 02/29/2016 09:00 PM, Jim Fehlig wrote:
> An expanded V2 of
>
> https://www.redhat.com/archives/libvir-list/2016-February/msg00140.html
>
> In V2, the  feature is extended with a state='on|off' attribute to
> allow differentiating the 'on' and 'off' states with not set (or hypervisor
> default).

Any comments about this approach? I essentially followed the same pattern as the
 and  hypervisor features.  Thanks!

Regards,
Jim

>
> Obviously post 1.3.2 material. See individual patches for details.
>
> Jim Fehlig (4):
>   conf: add 'state' attribute to  feature
>   xenconfig: change 'hap' setting to align with Xen behavior
>   Xen drivers: show hap enabled by default in capabilities
>   libxl: support enabling and disabling  feature
>
>  docs/formatdomain.html.in  |  6 ++-
>  docs/schemas/domaincommon.rng  |  6 ++-
>  src/conf/domain_conf.c |  4 +-
>  src/libxl/libxl_conf.c | 20 ++--
>  src/xen/xen_hypervisor.c   |  2 +-
>  src/xenconfig/xen_common.c | 14 ++---
>  .../test-disk-positional-parms-full.cfg|  1 -
>  .../test-disk-positional-parms-partial.cfg |  1 -
>  ...est-fullvirt-direct-kernel-boot-bogus-extra.cfg |  1 -
>  .../test-fullvirt-direct-kernel-boot-extra.cfg |  1 -
>  .../test-fullvirt-direct-kernel-boot.cfg   |  1 -
>  tests/xlconfigdata/test-fullvirt-multiusb.cfg  |  1 -
>  tests/xlconfigdata/test-fullvirt-nohap.cfg | 26 ++
>  tests/xlconfigdata/test-fullvirt-nohap.xml | 59 
> ++
>  tests/xlconfigdata/test-new-disk.cfg   |  1 -
>  tests/xlconfigdata/test-rbd-multihost-noauth.cfg   |  1 -
>  tests/xlconfigdata/test-spice-features.cfg |  1 -
>  tests/xlconfigdata/test-spice.cfg  |  1 -
>  tests/xlconfigdata/test-vif-rate.cfg   |  1 -
>  tests/xlconfigtest.c   |  1 +
>  tests/xmconfigdata/test-escape-paths.cfg   |  1 -
>  .../xmconfigdata/test-fullvirt-default-feature.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-force-hpet.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-force-nohpet.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-localtime.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-net-netfront.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-new-cdrom.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-nohap.cfg | 28 ++
>  tests/xmconfigdata/test-fullvirt-nohap.xml | 51 +++
>  tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg  |  1 -
>  .../test-fullvirt-serial-dev-2-ports.cfg   |  1 -
>  .../test-fullvirt-serial-dev-2nd-port.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-file.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-null.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-pipe.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-pty.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-stdio.cfg  |  1 -
>  .../test-fullvirt-serial-tcp-telnet.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-tcp.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-udp.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-unix.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-sound.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-usbmouse.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-usbtablet.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-utc.cfg   |  1 -
>  tests/xmconfigdata/test-no-source-cdrom.cfg|  1 -
>  tests/xmconfigdata/test-pci-devs.cfg   |  1 -
>  tests/xmconfigtest.c   |  1 +
>  48 files changed, 202 insertions(+), 52 deletions(-)
>  create mode 100644 tests/xlconfigdata/test-fullvirt-nohap.cfg
>  create mode 100644 tests/xlconfigdata/test-fullvirt-nohap.xml
>  create mode 100755 tests/xmconfigdata/test-fullvirt-nohap.cfg
>  create mode 100644 tests/xmconfigdata/test-fullvirt-nohap.xml
>

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

Re: [libvirt] [PATCH] schema: support 'default' cache mode

[Jim Fehlig]

Hi All,

Any comments about this small patch?

Regards,
Jim

On 02/29/2016 04:26 PM, Jim Fehlig wrote:
> The docs claims the cache attribute of the disk 
> element supports 'default' as one of its permissible values,
> but such configuration fails virt-xml-validate. Add 'default'
> as one of the cache attribute choices in domaincommon.rng.
>
> Signed-off-by: Jim Fehlig 
> ---
>  docs/schemas/domaincommon.rng | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 67af93a..d4e375f 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1596,6 +1596,7 @@
>
>  
>
> +default
>  none
>  writeback
>  writethrough

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

Re: [libvirt] [PATCH 10/24] hostdev: Remove explicit NULL checks

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

NULL checks are performed implicitly in the rest of the module,
including other allocations in the very same function.
---
  src/util/virhostdev.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


ACK.

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

Re: [libvirt] [PATCH 09/24] hostdev: Fix indentation

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

---
  src/util/virhostdev.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 5be61cd..48a44bc 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -833,9 +833,10 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr 
hostdev_mgr,
  virPCIDeviceGetUsedBy(activeDev, _drvname, 
_domname);
  if (STRNEQ_NULLABLE(drv_name, usedby_drvname) ||
  STRNEQ_NULLABLE(dom_name, usedby_domname)) {
-virPCIDeviceListDel(pcidevs, dev);
-continue;
-}
+
+virPCIDeviceListDel(pcidevs, dev);
+continue;
+}
  }
  
  VIR_DEBUG("Removing PCI device %s from active list",


ACK :-)

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

Re: [libvirt] [PATCH 08/24] hostdev: Remove false comment

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

---
  src/util/virhostdev.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index adc4eec..5be61cd 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -727,9 +727,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
  virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
  
  if (virPCIDeviceGetManaged(dev)) {

-/* NB: This doesn't actually re-bind to original driver, just
- * unbinds from the stub driver
- */
  VIR_DEBUG("Reattaching managed PCI device %s",
virPCIDeviceGetName(dev));
  ignore_value(virPCIDeviceReattach(dev,


ACK.

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

Re: [libvirt] [PATCH 07/24] hostdev: Make comments easier to change later

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

Replace the term "loop" with the more generic "step". This allows us
to be more flexible and eg. have a step that consists in a single
function call.

Don't include the number of steps in the first comment of the
function, so that we can add or remove steps without having to worry
about keeping that comment in sync.

For the same reason, remove the summary contained in that comment.

Clean up some weird vertical spacing while we're at it.
---
  src/util/virhostdev.c | 58 +++
  1 file changed, 26 insertions(+), 32 deletions(-)


ACK.


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

Re: [libvirt] [PATCH 06/24] tests: hostdev: Group test cases

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

Instead of considering each single step its own test case, create
high level test cases that reproduce a certain scenario.
---
  tests/virhostdevtest.c | 119 -
  1 file changed, 97 insertions(+), 22 deletions(-)


I like the idea, just have two comments:

1) would it maybe be useful to keep the individual tests, in order to 
make it simpler to determine which piece had broken in the case of a 
regression?


2) It looks like this only tests for legacy kvm (i.e. 
attaching/detaching pci-stub). It would be good to have it test for vfio 
as well (although maybe that can be saved for after we add support for 
using individual devices' driver_override to attach different drivers).


ACK as is, the comments are food for thought.



diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 8dc92f6..c0ab044 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -160,7 +160,7 @@ virHostdevHostSupportsPassthroughKVM(void)
  # endif
  
  static int

-testVirHostdevPreparePCIHostdevs_unmanaged(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevPreparePCIHostdevs_unmanaged(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -219,7 +219,7 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
  }
  
  static int

-testVirHostdevReAttachPCIHostdevs_unmanaged(const void *opaque 
ATTRIBUTE_UNUSED)
+testVirHostdevReAttachPCIHostdevs_unmanaged(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -253,7 +253,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
  }
  
  static int

-testVirHostdevPreparePCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevPreparePCIHostdevs_managed(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -304,7 +304,7 @@ testVirHostdevPreparePCIHostdevs_managed(const void *opaque 
ATTRIBUTE_UNUSED)
  }
  
  static int

-testVirHostdevReAttachPCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevReAttachPCIHostdevs_managed(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -338,7 +338,7 @@ testVirHostdevReAttachPCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
  }
  
  static int

-testVirHostdevDetachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevDetachPCINodeDevice(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -357,8 +357,9 @@ testVirHostdevDetachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
   cleanup:
  return ret;
  }
+
  static int
-testVirHostdevResetPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevResetPCINodeDevice(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -380,7 +381,7 @@ testVirHostdevResetPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
  }
  
  static int

-testVirHostdevReAttachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevReAttachPCINodeDevice(void)
  {
  int ret = -1;
  size_t active_count, inactive_count, i;
@@ -402,7 +403,7 @@ testVirHostdevReAttachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
  }
  
  static int

-testVirHostdevUpdateActivePCIHostdevs(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevUpdateActivePCIHostdevs(void)
  {
  int ret = -1;
  size_t active_count, inactive_count;
@@ -430,6 +431,91 @@ testVirHostdevUpdateActivePCIHostdevs(const void *opaque 
ATTRIBUTE_UNUSED)
  return ret;
  }
  
+/**

+ * testVirHostdevRoundtripUnmanaged:
+ * @opaque: unused
+ *
+ * Perform a roundtrip with unmanaged devices.
+ *
+ *   1. Detach devices from the host
+ *   2. Attach devices to the guest as unmanaged
+ *   3. Detach devices from the guest as unmanaged
+ *   4. Reattach devices to the host
+ */
+static int
+testVirHostdevRoundtripUnmanaged(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (testVirHostdevDetachPCINodeDevice() < 0)
+goto out;
+if (virHostdevHostSupportsPassthroughKVM()) {
+if (testVirHostdevPreparePCIHostdevs_unmanaged() < 0)
+goto out;
+if (testVirHostdevReAttachPCIHostdevs_unmanaged() < 0)
+goto out;
+}
+if (testVirHostdevReAttachPCINodeDevice() < 0)
+goto out;
+
+ret = 0;
+
+ out:
+return ret;
+}
+
+/**
+ * testVirHostdevRoundtripManaged:
+ * @opaque: unused
+ *
+ * Perform a roundtrip with managed devices.
+ *
+ *   1. Attach devices to the guest as managed
+ *   2. Detach devices from the guest as managed
+ */
+static int
+testVirHostdevRoundtripManaged(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (virHostdevHostSupportsPassthroughKVM()) {
+if (testVirHostdevPreparePCIHostdevs_managed() < 0)
+goto out;
+if (testVirHostdevReAttachPCIHostdevs_managed() < 0)
+goto out;
+}
+
+ret = 0;
+
+ out:
+

Re: [libvirt] [PATCH 01/10] secret: Move virSecretObj to secret_conf.h

[John Ferlan]

On 03/07/2016 10:50 AM, Peter Krempa wrote:
> On Wed, Mar 02, 2016 at 13:54:58 -0500, John Ferlan wrote:
>> To support being able to create a hashed secrets list, move the
>> virSecretObj to secret_conf.h so that secret_conf.c can at least find
>> the definition.
> 
> I don't think this is necessary. You still can create accessors and have
> the actual implementation of the struct private in the .c file. That way
> it's guaranteed that nobody will touch the fields directly accross the
> source files.
> 

Suffice to say there's still more work to do; however, short term I
think it needs to move so I can make some progress. I will have a
followup series that will move _virSecretObj into secret_conf.c.

The goal for this series was to build up the code to support a hashed
secret object while still having that linked list in the driver. Patch
10 was added to this series mainly since it was discussed in danpb's
review of my last set of secret driver changes. Moving the saving of the
config file and secret data file was next on the todo list.

I should note that the existing model has virDomainObj in
src/conf/domain_conf.h and virNetworkObj in src/conf/network_conf.h, so
virSecretObj wouldn't be unique...

I honestly didn't want another 15-20 patch series floating around. Make
some progress, share, get buy in, continue.  The question then becomes
does the rest of the series look good?

Tks -

John

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

Re: [libvirt] ruby-libvirt mixed storage migration

[Chris Lalancette]

On Mon, Mar 7, 2016 at 10:48 AM, Tim Förster - Hetzner Online AG <
tim.foers...@hetzner.de> wrote:

> Hi,
>
> rebuilding the gem is not quite easy. I am not able to find the actual
> gemspec. There are also some
>

The gemspec is in the git repository.  You should be able to clone git://
libvirt.org/ruby-libvirt.git, then run "rake gem" to get a new gem.


> missing features, which are available though virsh. Such as 'virsh
> domblklist' to find out the
> actual registered block targets or rate limiting with iotune the burst
> iops.
>
>
ruby-libvirt aims to expose the libvirt API through ruby, not the virsh
API.  Sometimes virsh provides additional functionality on top of the
libvirt API, which will not necessarily be implemented in ruby-libvirt.  In
this particular case, virsh domblklist parses virDomainGetXMLDesc() to
output a list of block devices.  virDomainGetXMLDesc() is supported by
ruby-libvirt, but domblklist itself would not be.  That being said, it
should be fairly straightforward to implement it in ruby by first calling
dom.xml_desc(), and then parsing the output with your favorite ruby XML
parsing library.

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

Re: [libvirt] [PATCH] ruby-libvirt: Don't crash in leases_wrap() by passing NULLs to rb_str_new2()

[Dan Williams]

On Thu, 2016-01-14 at 21:44 -0500, Chris Lalancette wrote:
> On Thu, Jan 14, 2016 at 9:38 PM, Chris Lalancette  com>
> wrote:
> 
> > 
> > Hi!
> > 
> > On Thu, Jan 14, 2016 at 2:56 PM, Dan Williams 
> > wrote:
> > 
> > > 
> > > On Thu, 2016-01-14 at 13:19 -0500, Laine Stump wrote:
> > > > 
> > > > On 01/14/2016 11:01 AM, Dan Williams wrote:
> > > > > 
> > > > > On Thu, 2016-01-07 at 11:12 -0600, Dan Williams wrote:
> > > > > > 
> > > > > > Not all lease values are mandatory, and when they aren't
> > > > > > supplied
> > > > > > by the libvirt driver they get set to NULL.  That makes
> > > > > > rb_str_new2() bail out.
> > > > > Ping?  Does this patch look OK or is there anything else I
> > > > > need to
> > > > > do
> > > > > with it?  Is the submission procure for ruby-libvirt
> > > > > different than
> > > > > normal libvirt?
> > > > As far as I can see, posting to libvir-list is the correct
> > > > thing for
> > > > ruby-libvirt patches, I think it's just that very few people
> > > > use it,
> > > > so
> > > > most of us don't feel comfortable ACKing anything for it.
> > > Thanks!  I'd bet a lot more people use it than you think, since
> > > it's a
> > > dependency of vagrant-libvirt by way of fog.  So you can't stand
> > > up a
> > > vagrant machine using libvirt without it...
> > > 
> > > 
> > Dan
> > > 
> > > 
> > > > 
> > > > It looks like Chris Lalancette is the maintainer and has been
> > > > the
> > > > author
> > > > of nearly every patch in the last couple years, so I'm Cc'ing
> > > > him to
> > > > be
> > > > sure it see it
> > 
> > For whatever reason, I didn't see your earlier patches.  I'll take
> > a look
> > and get back to you.
> > 
> 
> I've now applied the patch.  Thanks for the contribution!

Chris, any chance we could get a new ruby-libvirt point release with
the dhcp_leases fix patch in it?  The fog-libvirt patches to use that
functionality have now landed and people are confused that ruby-libvirt 
0.6.0 doesn't have all the necessary fixes.

Thanks,
Dan

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

Re: [libvirt] GSoC 2016

[Michal Privoznik]

On 07.03.2016 18:20, Peter Krempa wrote:
> On Mon, Mar 07, 2016 at 16:50:41 +0100, Martin Kletzander wrote:
>> Hi,
>>
>> it's good to hear that you are interested.  I'll give you the same hints
>> as I give others for now.  But before that, have you had a chance of
>> looking at the ideas page of libvirt?  Is any one of those ideas
>> appealing to you or do you have your own
>>
>> As for the ideas, look at our documentation, clone the repository [1],
>> try building it [2], running tests etc., look at the contributor
>> guidelines [2] and generally browse the site to know about some basics.
>> You can also browse the code on the web [4].  Then you can go through
>> the code and see how it all works; or the parts that interest you the
>> most, for now.
>>
>> Be sure to fill in the application before the deadline, the time window
>> is pretty short this year.
> 
> You should add the above to the wiki page with libvirt's GSOC info. It
> will be useful.
> 
> Additionally google's page contains link only to our web page and the
> email address. There's no link to the wiki including the project idea
> list.

How come! I see 'View Ideas List' button in the very top right corner.
It links to:

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

The design of GSoC pages has changed this year so it might be confusing
to some users to get all the information at the first glance.

Michal

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

Re: [libvirt] [PATCH 05/24] tests: hostdev: Add more checks on list size

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

Always call CHECK_LIST_COUNT() to check the size of both the active
and inactive devices list.
---
  tests/virhostdevtest.c | 34 --
  1 file changed, 28 insertions(+), 6 deletions(-)




ACK.

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

Re: [libvirt] [PATCH 04/24] tests: hostdev: Use size_t for count variables

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

virPCIDeviceListCount()'s return type is size_t, so variables that
store its return value should be of that type.
---
  tests/virhostdevtest.c | 24 +---
  1 file changed, 9 insertions(+), 15 deletions(-)



ACK.

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

Re: [libvirt] [PATCH 03/24] tests: hostdev: Declare count inside CHECK_LIST_COUNT()

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

This variable is only used internally by the macro, so it's better to
move its definition inside the macro as well.
---
  tests/virhostdevtest.c | 31 +--
  1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index febb2c9..77b119b 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -37,13 +37,16 @@
  
  VIR_LOG_INIT("tests.hostdevtest");
  
-# define CHECK_LIST_COUNT(list, cnt)\

-if ((count = virPCIDeviceListCount(list)) != cnt) { \
-virReportError(VIR_ERR_INTERNAL_ERROR,  \
-   "Unexpected count of items in " #list ": %d, "   \
-   "expecting %zu", count, (size_t) cnt);   \
-goto cleanup;   \
-}
+# define CHECK_LIST_COUNT(list, cnt)\
+do {\
+int count;  \
+if ((count = virPCIDeviceListCount(list)) != cnt) { \
+virReportError(VIR_ERR_INTERNAL_ERROR,  \
+   "Unexpected count of items in " #list ": %d, "   \
+   "expecting %zu", count, (size_t) cnt);   \
+goto cleanup;   \
+}   \
+} while (0)


The only suggestion I would have is to make "count" something less 
common, so that you're less likely to end up causing a compiler warning 
later if someone adds a variable called "count" to a function that uses 
this macro.


Otherwise ACK.

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

Re: [libvirt] [PATCH 02/24] tests: hostdev: Use better variable names

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

Change the extremely generic count1 and count2 to the more
descriptive active_count and inactive_count.
---
  tests/virhostdevtest.c | 82 +-
  1 file changed, 41 insertions(+), 41 deletions(-)



ACK.

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

Re: [libvirt] [PATCH 01/24] tests: hostdev: Remove magic numbers

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

When checking the number of devices added to a device list, use the
nhostdevs variable instead of its value, so that the test can keep
working even if more hostdevs are added.
---
  tests/virhostdevtest.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)


ACK.

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

Re: [libvirt] [PATCH] Fix minor typos in messages

[Andrea Bolognani]

On Mon, 2016-03-07 at 15:32 +0200, Yuri Chornoivan wrote:
> Hi,
> 
> Attached is a patch to fix minor typos in libvirt messages. Thanks for  
> fixing them in the repo.

Pushed, thanks for your contribution :)

> By the way, libvirt template has not been updated for almost 1 year on  
> Zanata. Are the translation moved into some new location?

Unfortunately I don't have the answer to this question, but I'm sure
someone else on the list will be able to explain what's going on...

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

[libvirt] [PATCH 14/24] hostdev: Look up devices using IDs when possible

[Andrea Bolognani]

When we want to look up a device in a device list and we already
have the IDs from another source, we can simply use
virPCIDeviceListFindByIDs() instead of creating a temporary device
object.
---
 src/util/virhostdev.c | 59 ---
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index e2accb4..fef7898 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -64,15 +64,11 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 {
 virPCIDevicePtr other;
 int ret = -1;
-virPCIDevicePtr pci = NULL;
 struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
 
-if (!(pci = virPCIDeviceNew(devAddr->domain, devAddr->bus,
-devAddr->slot, devAddr->function)))
-goto cleanup;
-
-other = virPCIDeviceListFind(helperData->hostdev_mgr->activePCIHostdevs,
- pci);
+other = 
virPCIDeviceListFindByIDs(helperData->hostdev_mgr->activePCIHostdevs,
+  devAddr->domain, devAddr->bus,
+  devAddr->slot, devAddr->function);
 if (other) {
 const char *other_drvname = NULL;
 const char *other_domname = NULL;
@@ -87,18 +83,17 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is in use by "
  "driver %s, domain %s"),
-   virPCIDeviceGetName(pci),
+   virPCIDeviceGetName(other),
other_drvname, other_domname);
 else
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is in use"),
-   virPCIDeviceGetName(pci));
+   virPCIDeviceGetName(other));
 goto cleanup;
 }
  iommu_owner:
 ret = 0;
  cleanup:
-virPCIDeviceFree(pci);
 return ret;
 }
 
@@ -669,7 +664,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 /* Step 8: Now set the original states for hostdev def */
 for (i = 0; i < nhostdevs; i++) {
 virPCIDevicePtr dev;
-virPCIDevicePtr pcidev;
 virDomainHostdevDefPtr hostdev = hostdevs[i];
 virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
 
@@ -678,24 +672,25 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
 continue;
 
-dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
-  pcisrc->addr.slot, pcisrc->addr.function);
+dev = virPCIDeviceListFindByIDs(pcidevs,
+pcisrc->addr.domain,
+pcisrc->addr.bus,
+pcisrc->addr.slot,
+pcisrc->addr.function);
 
 /* Appropriate values for the unbind_from_stub, remove_slot
  * and reprobe properties of the device were set earlier
  * by virPCIDeviceDetach() */
-VIR_DEBUG("Saving network configuration of PCI device %s",
-  virPCIDeviceGetName(dev));
-if ((pcidev = virPCIDeviceListFind(pcidevs, dev))) {
+if (dev) {
+VIR_DEBUG("Saving network configuration of PCI device %s",
+  virPCIDeviceGetName(dev));
 hostdev->origstates.states.pci.unbind_from_stub =
-virPCIDeviceGetUnbindFromStub(pcidev);
+virPCIDeviceGetUnbindFromStub(dev);
 hostdev->origstates.states.pci.remove_slot =
-virPCIDeviceGetRemoveSlot(pcidev);
+virPCIDeviceGetRemoveSlot(dev);
 hostdev->origstates.states.pci.reprobe =
-virPCIDeviceGetReprobe(pcidev);
+virPCIDeviceGetReprobe(dev);
 }
-
-virPCIDeviceFree(dev);
 }
 
 /* Step 9: Now steal all the devices from pcidevs */
@@ -857,18 +852,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 
 if (virHostdevIsPCINetDevice(hostdev)) {
 virDomainHostdevSubsysPCIPtr pcisrc = 
>source.subsys.u.pci;
-virPCIDevicePtr dev = NULL;
-dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
-  pcisrc->addr.slot, pcisrc->addr.function);
+virPCIDevicePtr dev;
+
+dev = virPCIDeviceListFindByIDs(pcidevs,
+pcisrc->addr.domain,
+pcisrc->addr.bus,
+pcisrc->addr.slot,
+pcisrc->addr.function);
+
 

[libvirt] [PATCH 02/24] tests: hostdev: Use better variable names

[Andrea Bolognani]

Change the extremely generic count1 and count2 to the more
descriptive active_count and inactive_count.
---
 tests/virhostdevtest.c | 82 +-
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index fa6a4fb..febb2c9 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -161,52 +161,52 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, count1, count2;
+int count, active_count, inactive_count;
 
 for (i = 0; i < nhostdevs; i++)
  hostdevs[i]->managed = false;
 
-count1 = virPCIDeviceListCount(mgr->activePCIHostdevs);
-count2 = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
+active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
+inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 
 /* Test normal functionality */
 VIR_DEBUG("Test 0 hostdevs");
 if (virHostdevPreparePCIDevices(mgr, drv_name, dom_name, uuid,
 NULL, 0, 0) < 0)
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
 
 /* Test unmanaged hostdevs */
 VIR_DEBUG("Test >=1 unmanaged hostdevs");
 if (virHostdevPreparePCIDevices(mgr, drv_name, dom_name, uuid,
 hostdevs, nhostdevs, 0) < 0)
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + nhostdevs);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2 - nhostdevs);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count + nhostdevs);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count - nhostdevs);
 
 /* Test conflict */
-count1 = virPCIDeviceListCount(mgr->activePCIHostdevs);
-count2 = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
+active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
+inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 VIR_DEBUG("Test: prepare same hostdevs for same driver/domain again");
 if (!virHostdevPreparePCIDevices(mgr, drv_name, dom_name, uuid,
  [0], 1, 0))
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 VIR_DEBUG("Test: prepare same hostdevs for same driver, diff domain 
again");
 if (!virHostdevPreparePCIDevices(mgr, drv_name, "test_domain1", uuid,
  [1], 1, 0))
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 VIR_DEBUG("Test: prepare same hostdevs for diff driver/domain again");
 if (!virHostdevPreparePCIDevices(mgr, "test_driver1", dom_name, uuid,
  [2], 1, 0))
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 ret = 0;
 
@@ -220,7 +220,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, count1, count2;
+int count, active_count, inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != false) {
@@ -229,18 +229,18 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 }
 }
 
-count1 = virPCIDeviceListCount(mgr->activePCIHostdevs);
-count2 = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
+active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
+inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 
 VIR_DEBUG("Test 0 hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0, NULL);
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
 
 VIR_DEBUG("Test >=1 unmanaged hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
   hostdevs, nhostdevs, NULL);
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 - nhostdevs);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2 + nhostdevs);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count - nhostdevs);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count + nhostdevs);
 
 ret = 0;
 
@@ -254,39 +254,39 @@ testVirHostdevPreparePCIHostdevs_managed(const void 
*opaque 

[libvirt] [PATCH 11/24] hostdev: Remove redundant check

[Andrea Bolognani]

If 'last_processed_hostdev_vf != -1' is false then, since the
loop counter 'i' starts at 0, 'i <= last_processed_hostdev_vf'
can't possibly be true and the loop body will never be executed.

Hence, the first check is completely redundant and can be safely
removed.
---
 src/util/virhostdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 098207e..f08502b 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -718,8 +718,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 }
 
  resetvfnetconfig:
-for (i = 0;
- last_processed_hostdev_vf != -1 && i <= last_processed_hostdev_vf; 
i++)
+for (i = 0; i <= last_processed_hostdev_vf; i++)
 virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr->stateDir, NULL);
 
  reattachdevs:
-- 
2.5.0

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

[libvirt] [PATCH 05/24] tests: hostdev: Add more checks on list size

[Andrea Bolognani]

Always call CHECK_LIST_COUNT() to check the size of both the active
and inactive devices list.
---
 tests/virhostdevtest.c | 34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index c1321b1..8dc92f6 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -177,6 +177,7 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 NULL, 0, 0) < 0)
 goto cleanup;
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 /* Test unmanaged hostdevs */
 VIR_DEBUG("Test >=1 unmanaged hostdevs");
@@ -236,6 +237,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 VIR_DEBUG("Test 0 hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0, NULL);
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 VIR_DEBUG("Test >=1 unmanaged hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
@@ -254,12 +256,13 @@ static int
 testVirHostdevPreparePCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t active_count, i;
+size_t active_count, inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++)
 hostdevs[i]->managed = true;
 
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
+inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 
 /* Test normal functionality */
 VIR_DEBUG("Test >=1 hostdevs");
@@ -267,26 +270,31 @@ testVirHostdevPreparePCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
  hostdevs, nhostdevs, 0) < 0)
 goto cleanup;
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count + nhostdevs);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 /* Test conflict */
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
+inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 VIR_DEBUG("Test: prepare same hostdevs for same driver/domain again");
 if (!virHostdevPreparePCIDevices(mgr, drv_name, dom_name, uuid,
   [0], 1, 0))
 goto cleanup;
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 VIR_DEBUG("Test: prepare same hostdevs for same driver, diff domain 
again");
 if (!virHostdevPreparePCIDevices(mgr, drv_name, "test_domain1", uuid,
   [1], 1, 0))
 goto cleanup;
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 VIR_DEBUG("Test: prepare same hostdevs for diff driver/domain again");
 if (!virHostdevPreparePCIDevices(mgr, "test_driver1", dom_name, uuid,
   [2], 1, 0))
 goto cleanup;
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 ret = 0;
 
@@ -299,7 +307,7 @@ static int
 testVirHostdevReAttachPCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t active_count, i;
+size_t active_count, inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != true) {
@@ -309,15 +317,18 @@ testVirHostdevReAttachPCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
 }
 
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
+inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 
 VIR_DEBUG("Test 0 hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0, NULL);
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 VIR_DEBUG("Test >=1 hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
   hostdevs, nhostdevs, NULL);
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count - nhostdevs);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 ret = 0;
 
@@ -330,12 +341,14 @@ static int
 testVirHostdevDetachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t inactive_count, i;
+size_t active_count, inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++) {
+active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
 if (virHostdevPCINodeDeviceDetach(mgr, dev[i]) < 0)
 goto cleanup;
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count);
 CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count + 1);
 }
 
@@ 

[libvirt] [PATCH 21/24] hostdev: Stop early if unmanaged devices have not been detached

[Andrea Bolognani]

Unmanaged devices, as the name suggests, are not detached
automatically from the host by libvirt before being attached to a
guest: it's the user's responsability to detach them manually
beforehand. If that preliminary step has not been performed, the
attach operation can't complete successfully.

Instead of relying on the lower layers to error out with cryptic
messages such as

  error: Failed to attach device from /tmp/hostdev.xml
  error: Path '/dev/vfio/12' is not accessible: No such file or directory

prevent the situation altogether and provide the user with a more
useful error message.
---
 src/util/virhostdev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 03c3445..d1529c5 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -576,6 +576,13 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
mgr->inactivePCIHostdevs) < 0)
 goto reattachdevs;
 } else {
+if (!virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("Unmanaged PCI device %s must be manually "
+   "detached from the host"),
+   virPCIDeviceGetName(pci));
+goto reattachdevs;
+}
 VIR_DEBUG("Not detaching unmanaged PCI device %s",
   virPCIDeviceGetName(pci));
 }
-- 
2.5.0

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

[libvirt] [PATCH 16/24] hostdev: Rename hostdev_mgr -> mgr

[Andrea Bolognani]

We're in the hostdev module, so mgr is not an ambiguous name, and
in fact it's already used in some cases. Switch all the code over.

Take the chance to shorten declaration of
virHostdevIsPCINodeDeviceUsedData structures.
---
 src/util/virhostdev.c | 155 +-
 1 file changed, 76 insertions(+), 79 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 67e6e7b..46a385c 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -55,7 +55,7 @@ static void virHostdevManagerDispose(void *obj);
 static virHostdevManagerPtr virHostdevManagerNew(void);
 
 struct virHostdevIsPCINodeDeviceUsedData {
-virHostdevManagerPtr hostdev_mgr;
+virHostdevManagerPtr mgr;
 const char *domainName;
 const bool usesVfio;
 };
@@ -66,7 +66,7 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 int ret = -1;
 struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
 
-other = 
virPCIDeviceListFindByIDs(helperData->hostdev_mgr->activePCIHostdevs,
+other = virPCIDeviceListFindByIDs(helperData->mgr->activePCIHostdevs,
   devAddr->domain, devAddr->bus,
   devAddr->slot, devAddr->function);
 if (other) {
@@ -426,7 +426,7 @@ virHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
  * For upgrade purpose:
  * To an existing VM on QEMU, the hostdev netconfig file is originally stored
  * in cfg->stateDir (/var/run/libvirt/qemu). Switch to new version, it uses new
- * location (hostdev_mgr->stateDir) but certainly will not find it. In this
+ * location (mgr->stateDir) but certainly will not find it. In this
  * case, try to find in the old state dir.
  */
 static int
@@ -475,7 +475,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev,
 }
 
 int
-virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
+virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 const char *drv_name,
 const char *dom_name,
 const unsigned char *uuid,
@@ -492,8 +492,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 if (!nhostdevs)
 return 0;
 
-virObjectLock(hostdev_mgr->activePCIHostdevs);
-virObjectLock(hostdev_mgr->inactivePCIHostdevs);
+virObjectLock(mgr->activePCIHostdevs);
+virObjectLock(mgr->inactivePCIHostdevs);
 
 if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs)))
 goto cleanup;
@@ -514,8 +514,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
 bool usesVfio = (virPCIDeviceGetStubDriver(dev) == 
VIR_PCI_STUB_DRIVER_VFIO);
-struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, dom_name,
- usesVfio};
+struct virHostdevIsPCINodeDeviceUsedData data = { mgr, dom_name, 
usesVfio };
 
 if (!usesVfio && !virPCIDeviceIsAssignable(dev, strict_acs_check)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
@@ -548,8 +547,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 VIR_DEBUG("Detaching managed PCI device %s",
   virPCIDeviceGetName(dev));
 if (virPCIDeviceDetach(dev,
-   hostdev_mgr->activePCIHostdevs,
-   hostdev_mgr->inactivePCIHostdevs) < 0)
+   mgr->activePCIHostdevs,
+   mgr->inactivePCIHostdevs) < 0)
 goto reattachdevs;
 } else {
 VIR_DEBUG("Not detaching unmanaged PCI device %s",
@@ -566,8 +565,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
 VIR_DEBUG("Resetting PCI device %s", virPCIDeviceGetName(dev));
-if (virPCIDeviceReset(dev, hostdev_mgr->activePCIHostdevs,
-  hostdev_mgr->inactivePCIHostdevs) < 0)
+if (virPCIDeviceReset(dev, mgr->activePCIHostdevs,
+  mgr->inactivePCIHostdevs) < 0)
 goto reattachdevs;
 }
 
@@ -578,7 +577,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
  if (!virHostdevIsPCINetDevice(hostdev))
  continue;
  if (virHostdevNetConfigReplace(hostdev, uuid,
-hostdev_mgr->stateDir) < 0) {
+mgr->stateDir) < 0) {
  goto resetvfnetconfig;
  }
  last_processed_hostdev_vf = i;
@@ -590,7 +589,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 
 VIR_DEBUG("Adding PCI device %s to active list",
   

[libvirt] [PATCH 22/24] hostdev: Streamline device ownership tracking

[Andrea Bolognani]

After this patch, ownership of virPCIDevice instances is very easy
to keep track of: for each host PCI device, the only instance that
actually matters is the one inside one of the bookkeeping list.

Whenever some operation needs to be performed on a PCI device, the
actual device is looked up first; when this is not the case, a
comment explains the reason.
---
 src/util/virhostdev.c | 118 +++---
 1 file changed, 65 insertions(+), 53 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index d1529c5..b2f54cd 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -618,28 +618,22 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  last_processed_hostdev_vf = i;
 }
 
-/* Step 5: Now mark all the devices as active */
+/* Step 5: Move devices from the inactive list to the active list */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
+virPCIDevicePtr actual;
 
-VIR_DEBUG("Adding PCI device %s to active list",
+VIR_DEBUG("Removing PCI device %s from inactive list",
   virPCIDeviceGetName(pci));
-if (virPCIDeviceListAdd(mgr->activePCIHostdevs, pci) < 0)
-goto inactivedevs;
-}
-
-/* Step 6: Now remove the devices from inactive list. */
-for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
-virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
+actual = virPCIDeviceListSteal(mgr->inactivePCIHostdevs, pci);
 
-VIR_DEBUG("Removing PCI device %s from inactive list",
+VIR_DEBUG("Adding PCI device %s to active list",
   virPCIDeviceGetName(pci));
-virPCIDeviceListDel(mgr->inactivePCIHostdevs, pci);
+if (!actual || virPCIDeviceListAdd(mgr->activePCIHostdevs, actual) < 0)
+goto inactivedevs;
 }
 
-/* Step 7: Now set the used_by_domain of the device in
- * activePCIHostdevs as domain name.
- */
+/* Step 6: Set driver and domain information */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci, actual;
 
@@ -655,7 +649,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 virPCIDeviceSetUsedBy(actual, drv_name, dom_name);
 }
 
-/* Step 8: Now set the original states for hostdev def */
+/* Step 7: Now set the original states for hostdev def */
 for (i = 0; i < nhostdevs; i++) {
 virPCIDevicePtr actual;
 virDomainHostdevDefPtr hostdev = hostdevs[i];
@@ -690,23 +684,25 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 }
 }
 
-/* Step 9: Now steal all the devices from pcidevs */
-while (virPCIDeviceListCount(pcidevs) > 0)
-virPCIDeviceListStealIndex(pcidevs, 0);
-
 ret = 0;
 goto cleanup;
 
  inactivedevs:
-/* Only steal all the devices from activePCIHostdevs. We will
- * free them in virObjectUnref().
- */
+/* Move devices back to the inactive list so that they can be
+ * processed properly below (reattachdevs label) */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
+virPCIDevicePtr actual;
 
 VIR_DEBUG("Removing PCI device %s from active list",
   virPCIDeviceGetName(pci));
-virPCIDeviceListSteal(mgr->activePCIHostdevs, pci);
+if (!(actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci)))
+continue;
+
+VIR_DEBUG("Adding PCI device %s to inactive list",
+  virPCIDeviceGetName(pci));
+if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0)
+continue;
 }
 
  resetvfnetconfig:
@@ -716,16 +712,22 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  reattachdevs:
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
+virPCIDevicePtr actual;
 
-if (virPCIDeviceGetManaged(pci)) {
+/* We need to look up the actual device because that's what
+ * virPCIDeviceReattach() exepects as its argument */
+if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
+continue;
+
+if (virPCIDeviceGetManaged(actual)) {
 VIR_DEBUG("Reattaching managed PCI device %s",
-  virPCIDeviceGetName(pci));
-ignore_value(virPCIDeviceReattach(pci,
+  virPCIDeviceGetName(actual));
+ignore_value(virPCIDeviceReattach(actual,
   mgr->activePCIHostdevs,
   mgr->inactivePCIHostdevs));
 } else {
 VIR_DEBUG("Not reattaching unmanaged PCI device %s",
-  virPCIDeviceGetName(pci));
+  virPCIDeviceGetName(actual));
 }
  

[libvirt] [PATCH 08/24] hostdev: Remove false comment

[Andrea Bolognani]

---
 src/util/virhostdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index adc4eec..5be61cd 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -727,9 +727,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
 if (virPCIDeviceGetManaged(dev)) {
-/* NB: This doesn't actually re-bind to original driver, just
- * unbinds from the stub driver
- */
 VIR_DEBUG("Reattaching managed PCI device %s",
   virPCIDeviceGetName(dev));
 ignore_value(virPCIDeviceReattach(dev,
-- 
2.5.0

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

[libvirt] [PATCH 23/24] hostdev: Use actual device when reattaching

[Andrea Bolognani]

Instead of forcing the values for the unbind_from_stub, remove_slot
and reprobe properties, look up the actual device and use that when
calling virPCIDeviceReattach().

This ensures the device is restored to its original state after
reattach: for example, if it was not bound to any driver before
detach, it will not be bound forcefully during reattach.
---
 src/util/virhostdev.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index b2f54cd..86a0331 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1613,6 +1613,7 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr mgr,
 virPCIDevicePtr pci)
 {
 struct virHostdevIsPCINodeDeviceUsedData data = { mgr, NULL, false };
+virPCIDevicePtr actual;
 int ret = -1;
 
 virObjectLock(mgr->activePCIHostdevs);
@@ -1621,11 +1622,12 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr 
mgr,
 if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), ))
 goto cleanup;
 
-virPCIDeviceSetUnbindFromStub(pci, true);
-virPCIDeviceSetRemoveSlot(pci, true);
-virPCIDeviceSetReprobe(pci, true);
+/* We need to look up the actual device because that's what
+ * virPCIDeviceReattach() expects as its argument */
+if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
+goto cleanup;
 
-if (virPCIDeviceReattach(pci, mgr->activePCIHostdevs,
+if (virPCIDeviceReattach(actual, mgr->activePCIHostdevs,
  mgr->inactivePCIHostdevs) < 0)
 goto cleanup;
 
-- 
2.5.0

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

[libvirt] [PATCH 20/24] hostdev: Save netdev configuration of actual device

[Andrea Bolognani]

We might be just fine looking up the information in pcidevs, but
it wouldn't save us any trouble and it's better to be consistent.
---
 src/util/virhostdev.c | 41 ++---
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index a431f0a..03c3445 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -650,7 +650,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 
 /* Step 8: Now set the original states for hostdev def */
 for (i = 0; i < nhostdevs; i++) {
-virPCIDevicePtr pci;
+virPCIDevicePtr actual;
 virDomainHostdevDefPtr hostdev = hostdevs[i];
 virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
 
@@ -659,24 +659,27 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
 continue;
 
-pci = virPCIDeviceListFindByIDs(pcidevs,
-pcisrc->addr.domain,
-pcisrc->addr.bus,
-pcisrc->addr.slot,
-pcisrc->addr.function);
+/* We need to look up the actual device because it's the one
+ * that contains the information we care about (unbind_from_stub,
+ * remove_slot, reprobe) */
+actual = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs,
+   pcisrc->addr.domain,
+   pcisrc->addr.bus,
+   pcisrc->addr.slot,
+   pcisrc->addr.function);
 
 /* Appropriate values for the unbind_from_stub, remove_slot
  * and reprobe properties of the device were set earlier
  * by virPCIDeviceDetach() */
-if (pci) {
+if (actual) {
 VIR_DEBUG("Saving network configuration of PCI device %s",
-  virPCIDeviceGetName(pci));
+  virPCIDeviceGetName(actual));
 hostdev->origstates.states.pci.unbind_from_stub =
-virPCIDeviceGetUnbindFromStub(pci);
+virPCIDeviceGetUnbindFromStub(actual);
 hostdev->origstates.states.pci.remove_slot =
-virPCIDeviceGetRemoveSlot(pci);
+virPCIDeviceGetRemoveSlot(actual);
 hostdev->origstates.states.pci.reprobe =
-virPCIDeviceGetReprobe(pci);
+virPCIDeviceGetReprobe(actual);
 }
 }
 
@@ -844,17 +847,17 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
 
 if (virHostdevIsPCINetDevice(hostdev)) {
 virDomainHostdevSubsysPCIPtr pcisrc = 
>source.subsys.u.pci;
-virPCIDevicePtr pci;
+virPCIDevicePtr actual;
 
-pci = virPCIDeviceListFindByIDs(pcidevs,
-pcisrc->addr.domain,
-pcisrc->addr.bus,
-pcisrc->addr.slot,
-pcisrc->addr.function);
+actual = virPCIDeviceListFindByIDs(mgr->inactivePCIHostdevs,
+   pcisrc->addr.domain,
+   pcisrc->addr.bus,
+   pcisrc->addr.slot,
+   pcisrc->addr.function);
 
-if (pci) {
+if (actual) {
 VIR_DEBUG("Restoring network configuration of PCI device %s",
-  virPCIDeviceGetName(pci));
+  virPCIDeviceGetName(actual));
 virHostdevNetConfigRestore(hostdev, mgr->stateDir,
oldStateDir);
 }
-- 
2.5.0

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

[libvirt] [PATCH 10/24] hostdev: Remove explicit NULL checks

[Andrea Bolognani]

NULL checks are performed implicitly in the rest of the module,
including other allocations in the very same function.
---
 src/util/virhostdev.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 48a44bc..098207e 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -142,16 +142,16 @@ virHostdevManagerNew(void)
 if (!(hostdevMgr = virObjectNew(virHostdevManagerClass)))
 return NULL;
 
-if ((hostdevMgr->activePCIHostdevs = virPCIDeviceListNew()) == NULL)
+if (!(hostdevMgr->activePCIHostdevs = virPCIDeviceListNew()))
 goto error;
 
-if ((hostdevMgr->activeUSBHostdevs = virUSBDeviceListNew()) == NULL)
+if (!(hostdevMgr->activeUSBHostdevs = virUSBDeviceListNew()))
 goto error;
 
-if ((hostdevMgr->inactivePCIHostdevs = virPCIDeviceListNew()) == NULL)
+if (!(hostdevMgr->inactivePCIHostdevs = virPCIDeviceListNew()))
 goto error;
 
-if ((hostdevMgr->activeSCSIHostdevs = virSCSIDeviceListNew()) == NULL)
+if (!(hostdevMgr->activeSCSIHostdevs = virSCSIDeviceListNew()))
 goto error;
 
 if (privileged) {
-- 
2.5.0

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

[libvirt] [PATCH 06/24] tests: hostdev: Group test cases

[Andrea Bolognani]

Instead of considering each single step its own test case, create
high level test cases that reproduce a certain scenario.
---
 tests/virhostdevtest.c | 119 -
 1 file changed, 97 insertions(+), 22 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 8dc92f6..c0ab044 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -160,7 +160,7 @@ virHostdevHostSupportsPassthroughKVM(void)
 # endif
 
 static int
-testVirHostdevPreparePCIHostdevs_unmanaged(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevPreparePCIHostdevs_unmanaged(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -219,7 +219,7 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 }
 
 static int
-testVirHostdevReAttachPCIHostdevs_unmanaged(const void *opaque 
ATTRIBUTE_UNUSED)
+testVirHostdevReAttachPCIHostdevs_unmanaged(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -253,7 +253,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 }
 
 static int
-testVirHostdevPreparePCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevPreparePCIHostdevs_managed(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -304,7 +304,7 @@ testVirHostdevPreparePCIHostdevs_managed(const void *opaque 
ATTRIBUTE_UNUSED)
 }
 
 static int
-testVirHostdevReAttachPCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevReAttachPCIHostdevs_managed(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -338,7 +338,7 @@ testVirHostdevReAttachPCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
 }
 
 static int
-testVirHostdevDetachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevDetachPCINodeDevice(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -357,8 +357,9 @@ testVirHostdevDetachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
  cleanup:
 return ret;
 }
+
 static int
-testVirHostdevResetPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevResetPCINodeDevice(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -380,7 +381,7 @@ testVirHostdevResetPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
 }
 
 static int
-testVirHostdevReAttachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevReAttachPCINodeDevice(void)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -402,7 +403,7 @@ testVirHostdevReAttachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
 }
 
 static int
-testVirHostdevUpdateActivePCIHostdevs(const void *opaque ATTRIBUTE_UNUSED)
+testVirHostdevUpdateActivePCIHostdevs(void)
 {
 int ret = -1;
 size_t active_count, inactive_count;
@@ -430,6 +431,91 @@ testVirHostdevUpdateActivePCIHostdevs(const void *opaque 
ATTRIBUTE_UNUSED)
 return ret;
 }
 
+/**
+ * testVirHostdevRoundtripUnmanaged:
+ * @opaque: unused
+ *
+ * Perform a roundtrip with unmanaged devices.
+ *
+ *   1. Detach devices from the host
+ *   2. Attach devices to the guest as unmanaged
+ *   3. Detach devices from the guest as unmanaged
+ *   4. Reattach devices to the host
+ */
+static int
+testVirHostdevRoundtripUnmanaged(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (testVirHostdevDetachPCINodeDevice() < 0)
+goto out;
+if (virHostdevHostSupportsPassthroughKVM()) {
+if (testVirHostdevPreparePCIHostdevs_unmanaged() < 0)
+goto out;
+if (testVirHostdevReAttachPCIHostdevs_unmanaged() < 0)
+goto out;
+}
+if (testVirHostdevReAttachPCINodeDevice() < 0)
+goto out;
+
+ret = 0;
+
+ out:
+return ret;
+}
+
+/**
+ * testVirHostdevRoundtripManaged:
+ * @opaque: unused
+ *
+ * Perform a roundtrip with managed devices.
+ *
+ *   1. Attach devices to the guest as managed
+ *   2. Detach devices from the guest as managed
+ */
+static int
+testVirHostdevRoundtripManaged(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (virHostdevHostSupportsPassthroughKVM()) {
+if (testVirHostdevPreparePCIHostdevs_managed() < 0)
+goto out;
+if (testVirHostdevReAttachPCIHostdevs_managed() < 0)
+goto out;
+}
+
+ret = 0;
+
+ out:
+return ret;
+}
+
+/**
+ * testVirHostdevOther:
+ * @opaque: unused
+ *
+ * Perform other operations on devices.
+ *
+ *   1. Reset devices
+ *   2. Update list of active devices
+ */
+static int
+testVirHostdevOther(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (testVirHostdevResetPCINodeDevice() < 0)
+goto out;
+if (testVirHostdevUpdateActivePCIHostdevs() < 0)
+goto out;
+
+ret = 0;
+
+ out:
+return ret;
+}
+
 # define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
 
 static int
@@ -460,20 +546,9 @@ mymain(void)
 if (myInit() < 0)
 fprintf(stderr, "Init data 

[libvirt] [PATCH 15/24] hostdev: Remove virHostdevGetActivePCIHostDeviceList()

[Andrea Bolognani]

virHostdevGetPCIHostDeviceList() is similar but does not filter out
devices that are not in the active list; that said, we are looking
up the device in the active list just a few lines after anyway, so
we might as well just keep a single function around.

This also helps stress the fact the objects contained in pcidevs are
only for looking up the actual devices, which is something later
commits will make even more explicit.
---
 src/util/virhostdev.c | 50 --
 1 file changed, 4 insertions(+), 46 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index fef7898..67e6e7b 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -245,49 +245,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr 
*hostdevs, int nhostdevs)
 }
 
 
-/*
- * virHostdevGetActivePCIHostDeviceList - make a new list with a *copy* of
- *   every virPCIDevice object that is found on the activePCIHostdevs
- *   list *and* is in the hostdev list for this domain.
- *
- * Return the new list, or NULL if there was a failure.
- *
- * Pre-condition: activePCIHostdevs is locked
- */
-static virPCIDeviceListPtr
-virHostdevGetActivePCIHostDeviceList(virHostdevManagerPtr mgr,
- virDomainHostdevDefPtr *hostdevs,
- int nhostdevs)
-{
-virPCIDeviceListPtr list;
-size_t i;
-
-if (!(list = virPCIDeviceListNew()))
-return NULL;
-
-for (i = 0; i < nhostdevs; i++) {
-virDomainHostdevDefPtr hostdev = hostdevs[i];
-virDevicePCIAddressPtr addr;
-virPCIDevicePtr activeDev;
-
-if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
-continue;
-if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
-continue;
-
-addr = >source.subsys.u.pci.addr;
-activeDev = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs,
-  addr->domain, addr->bus,
-  addr->slot, addr->function);
-if (activeDev && virPCIDeviceListAddCopy(list, activeDev) < 0) {
-virObjectUnref(list);
-return NULL;
-}
-}
-
-return list;
-}
-
 static int
 virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev,
char **sysfs_path)
@@ -800,9 +757,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virObjectLock(hostdev_mgr->activePCIHostdevs);
 virObjectLock(hostdev_mgr->inactivePCIHostdevs);
 
-if (!(pcidevs = virHostdevGetActivePCIHostDeviceList(hostdev_mgr,
- hostdevs,
- nhostdevs))) {
+if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs))) {
 virErrorPtr err = virGetLastError();
 VIR_ERROR(_("Failed to allocate PCI device list: %s"),
   err ? err->message : _("unknown error"));
@@ -832,6 +787,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDeviceListDel(pcidevs, dev);
 continue;
 }
+} else {
+virPCIDeviceListDel(pcidevs, dev);
+continue;
 }
 
 VIR_DEBUG("Removing PCI device %s from active list",
-- 
2.5.0

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

[libvirt] [PATCH 00/24] hostdev: Cleanups and fixes

[Andrea Bolognani]

Part of an ongoing quest to fix:

  libvirt management of VFIO devices can lead to host crashes
  https://bugzilla.redhat.com/show_bug.cgi?id=1272300

Previous episode:

  [PATCH v2 0/9] PCI hostdev refactoring
  https://www.redhat.com/archives/libvir-list/2016-January/msg01066.html

The plan:

  Once this is in, rebase the series mentioned above on top of it;
  hopefully it will be smaller and easier to review. Once *that*
  is in, rebase / rewrite the actual bug fix - probably the latter,
  because the code has changed a lot since I posted it

In this episode:

  01-06  Cleanups in the test cases; mostly unrelated, but commit
 24 depends on them

  07-17  More cleanups and (I believe) farily uncontroversial
 changes, aimed at making the actual changes either
 smaller or more obvious

  18-19  Change in variable names that make the intended semantics
 clearer, which should in turn make the follow-up changes
 easier to review

  Up until here there should be basically no change in behavior

 20  Bug fix

 21  Improvement to error reporting

 22  The whole point of this series, and also the commit
 where it's more likely that I might have gotten something
 wrong :) I haven't been able to figure out a way to split
 it into smaller chunks, sorry

 23 Another bug fix

 24 Test the changes made by the rest of the series

Cheers.


Andrea Bolognani (24):
  tests: hostdev: Remove magic numbers
  tests: hostdev: Use better variable names
  tests: hostdev: Declare count inside CHECK_LIST_COUNT()
  tests: hostdev: Use size_t for count variables
  tests: hostdev: Add more checks on list size
  tests: hostdev: Group test cases
  hostdev: Make comments easier to change later
  hostdev: Remove false comment
  hostdev: Fix indentation
  hostdev: Remove explicit NULL checks
  hostdev: Remove redundant check
  hostdev: virHostdevIsPCINetDevice() should return a bool
  hostdev: Change argument order for virHostdevReattachPCIDevice()
  hostdev: Look up devices using IDs when possible
  hostdev: Remove virHostdevGetActivePCIHostDeviceList()
  hostdev: Rename hostdev_mgr -> mgr
  hostdev: Rename usesVfio -> usesVFIO
  hostdev: Use consistent variable names
  hostdev: Add more comments
  hostdev: Save netdev configuration of actual device
  hostdev: Stop early if unmanaged devices have not been detached
  hostdev: Streamline device ownership tracking
  hostdev: Use actual device when reattaching
  tests: hostdev: Add more tests

 src/util/virhostdev.c  | 660 +
 tests/virhostdevtest.c | 320 ++--
 2 files changed, 576 insertions(+), 404 deletions(-)

-- 
2.5.0

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

[libvirt] [PATCH 13/24] hostdev: Change argument order for virHostdevReattachPCIDevice()

[Andrea Bolognani]

The new order aligns better with the virHostdev prefix.
---
 src/util/virhostdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index d3e15b7..e2accb4 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -750,7 +750,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
  * are locked
  */
 static void
-virHostdevReattachPCIDevice(virPCIDevicePtr dev, virHostdevManagerPtr mgr)
+virHostdevReattachPCIDevice(virHostdevManagerPtr mgr,
+virPCIDevicePtr dev)
 {
 /* If the device is not managed and was attached to guest
  * successfully, it must have been inactive.
@@ -890,7 +891,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr 
hostdev_mgr,
  */
 while (virPCIDeviceListCount(pcidevs) > 0) {
 virPCIDevicePtr dev = virPCIDeviceListStealIndex(pcidevs, 0);
-virHostdevReattachPCIDevice(dev, hostdev_mgr);
+virHostdevReattachPCIDevice(hostdev_mgr, dev);
 }
 
 virObjectUnref(pcidevs);
-- 
2.5.0

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

[libvirt] [PATCH 12/24] hostdev: virHostdevIsPCINetDevice() should return a bool

[Andrea Bolognani]

The only possible return values are true and false, so the return
type should be bool instead of int.
---
 src/util/virhostdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index f08502b..d3e15b7 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -354,7 +354,7 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char 
**linkdev,
 }
 
 
-static int
+static bool
 virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev)
 {
 return hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-- 
2.5.0

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

[libvirt] [PATCH 17/24] hostdev: Rename usesVfio -> usesVFIO

[Andrea Bolognani]

Acronyms should be written in all caps.
---
 src/util/virhostdev.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 46a385c..a7fb8b1 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -57,7 +57,7 @@ static virHostdevManagerPtr virHostdevManagerNew(void);
 struct virHostdevIsPCINodeDeviceUsedData {
 virHostdevManagerPtr mgr;
 const char *domainName;
-const bool usesVfio;
+const bool usesVFIO;
 };
 
 static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void 
*opaque)
@@ -74,7 +74,7 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 const char *other_domname = NULL;
 virPCIDeviceGetUsedBy(other, _drvname, _domname);
 
-if (helperData->usesVfio &&
+if (helperData->usesVFIO &&
 (other_domname && helperData->domainName) &&
 (STREQ(other_domname, helperData->domainName)))
 goto iommu_owner;
@@ -513,10 +513,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
-bool usesVfio = (virPCIDeviceGetStubDriver(dev) == 
VIR_PCI_STUB_DRIVER_VFIO);
-struct virHostdevIsPCINodeDeviceUsedData data = { mgr, dom_name, 
usesVfio };
+bool usesVFIO = (virPCIDeviceGetStubDriver(dev) == 
VIR_PCI_STUB_DRIVER_VFIO);
+struct virHostdevIsPCINodeDeviceUsedData data = { mgr, dom_name, 
usesVFIO };
 
-if (!usesVfio && !virPCIDeviceIsAssignable(dev, strict_acs_check)) {
+if (!usesVFIO && !virPCIDeviceIsAssignable(dev, strict_acs_check)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is not assignable"),
virPCIDeviceGetName(dev));
@@ -529,7 +529,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  * across guests.
  */
 devAddr = virPCIDeviceGetAddress(dev);
-if (usesVfio) {
+if (usesVFIO) {
 if (virPCIDeviceAddressIOMMUGroupIterate(devAddr,
  
virHostdevIsPCINodeDeviceUsed,
  ) < 0)
-- 
2.5.0

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

[libvirt] [PATCH 03/24] tests: hostdev: Declare count inside CHECK_LIST_COUNT()

[Andrea Bolognani]

This variable is only used internally by the macro, so it's better to
move its definition inside the macro as well.
---
 tests/virhostdevtest.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index febb2c9..77b119b 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -37,13 +37,16 @@
 
 VIR_LOG_INIT("tests.hostdevtest");
 
-# define CHECK_LIST_COUNT(list, cnt)\
-if ((count = virPCIDeviceListCount(list)) != cnt) { \
-virReportError(VIR_ERR_INTERNAL_ERROR,  \
-   "Unexpected count of items in " #list ": %d, "   \
-   "expecting %zu", count, (size_t) cnt);   \
-goto cleanup;   \
-}
+# define CHECK_LIST_COUNT(list, cnt)\
+do {\
+int count;  \
+if ((count = virPCIDeviceListCount(list)) != cnt) { \
+virReportError(VIR_ERR_INTERNAL_ERROR,  \
+   "Unexpected count of items in " #list ": %d, "   \
+   "expecting %zu", count, (size_t) cnt);   \
+goto cleanup;   \
+}   \
+} while (0)
 
 # define TEST_STATE_DIR abs_builddir "/hostdevmgr"
 static const char *drv_name = "test_driver";
@@ -161,7 +164,7 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count, inactive_count;
+int active_count, inactive_count;
 
 for (i = 0; i < nhostdevs; i++)
  hostdevs[i]->managed = false;
@@ -220,7 +223,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count, inactive_count;
+int active_count, inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != false) {
@@ -254,7 +257,7 @@ testVirHostdevPreparePCIHostdevs_managed(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count;
+int active_count;
 
 for (i = 0; i < nhostdevs; i++)
 hostdevs[i]->managed = true;
@@ -300,7 +303,7 @@ testVirHostdevReAttachPCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count;
+int active_count;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != true) {
@@ -332,7 +335,7 @@ testVirHostdevDetachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, inactive_count;
+int inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
@@ -369,7 +372,7 @@ testVirHostdevReAttachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, inactive_count;
+int inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
@@ -389,7 +392,7 @@ static int
 testVirHostdevUpdateActivePCIHostdevs(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-int count, active_count;
+int active_count;
 
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
 
-- 
2.5.0

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

[libvirt] [PATCH 01/24] tests: hostdev: Remove magic numbers

[Andrea Bolognani]

When checking the number of devices added to a device list, use the
nhostdevs variable instead of its value, so that the test can keep
working even if more hostdevs are added.
---
 tests/virhostdevtest.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index cadb66a..fa6a4fb 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -181,8 +181,8 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 if (virHostdevPreparePCIDevices(mgr, drv_name, dom_name, uuid,
 hostdevs, nhostdevs, 0) < 0)
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + 3);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2 - 3);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + nhostdevs);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2 - nhostdevs);
 
 /* Test conflict */
 count1 = virPCIDeviceListCount(mgr->activePCIHostdevs);
@@ -239,8 +239,8 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 VIR_DEBUG("Test >=1 unmanaged hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
   hostdevs, nhostdevs, NULL);
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 - 3);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2 + 3);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 - nhostdevs);
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, count2 + nhostdevs);
 
 ret = 0;
 
@@ -266,7 +266,7 @@ testVirHostdevPreparePCIHostdevs_managed(const void *opaque 
ATTRIBUTE_UNUSED)
 if (virHostdevPreparePCIDevices(mgr, drv_name, dom_name, uuid,
  hostdevs, nhostdevs, 0) < 0)
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + 3);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + nhostdevs);
 
 /* Test conflict */
 count1 = virPCIDeviceListCount(mgr->activePCIHostdevs);
@@ -318,7 +318,7 @@ testVirHostdevReAttachPCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
 VIR_DEBUG("Test >=1 hostdevs");
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
   hostdevs, nhostdevs, NULL);
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 - 3);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 - nhostdevs);
 
 ret = 0;
 
@@ -403,7 +403,7 @@ testVirHostdevUpdateActivePCIHostdevs(const void *opaque 
ATTRIBUTE_UNUSED)
 if (virHostdevUpdateActivePCIDevices(mgr, hostdevs, nhostdevs,
  drv_name, dom_name) < 0)
 goto cleanup;
-CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + 3);
+CHECK_LIST_COUNT(mgr->activePCIHostdevs, count1 + nhostdevs);
 
 ret = 0;
 
-- 
2.5.0

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

[libvirt] [PATCH 19/24] hostdev: Add more comments

[Andrea Bolognani]

These comments explain the difference between a virPCIDevice
instance used for lookups and an actual device instance; some
information is also provided for specific uses.
---
 src/util/virhostdev.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 10d1c1a..a431f0a 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -60,6 +60,27 @@ struct virHostdevIsPCINodeDeviceUsedData {
 const bool usesVFIO;
 };
 
+/* This module makes heavy use of bookkeeping lists, contained inside a
+ * virHostdevManager instance, to keep track of the devices' status. To make
+ * it easy to spot potential ownership errors when moving devices from one
+ * list to the other, variable names should comply with the following
+ * conventions when it comes to virPCIDevice and virPCIDeviceList instances:
+ *
+ *   pci - a short-lived virPCIDevice whose purpose is usually just to look
+ * up the actual PCI device in one of the bookkeeping lists; basically
+ * little more than a fancy virPCIDeviceAddress
+ *
+ *   pcidevs - a list containing a bunch of the above
+ *
+ *   actual - a virPCIDevice instance that has either been retrieved from one
+ *of the bookkeeping lists, or is intended to be added or copied
+ *to one at some point
+ *
+ * Passing an 'actual' to a function that requires a 'pci' is fine, but the
+ * opposite is usually not true; as a rule of thumb, functions in the virpci
+ * module usually expect an 'actual'. Even with these conventions in place,
+ * adding comments to highlight ownership-related issues is recommended */
+
 static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void 
*opaque)
 {
 virPCIDevicePtr actual;
@@ -544,6 +565,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
 
 if (virPCIDeviceGetManaged(pci)) {
+/* We can't look up the actual device because it has not been
+ * created yet: virPCIDeviceDetach() will insert a copy of 'pci'
+ * into the list of inactive devices, and that copy will be the
+ * actual device going forward */
 VIR_DEBUG("Detaching managed PCI device %s",
   virPCIDeviceGetName(pci));
 if (virPCIDeviceDetach(pci,
@@ -564,6 +589,9 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
 
+/* We can avoid looking up the actual device here, because performing
+ * a PCI reset on a device doesn't require any information other than
+ * the address, which 'pci' already contains */
 VIR_DEBUG("Resetting PCI device %s", virPCIDeviceGetName(pci));
 if (virPCIDeviceReset(pci, mgr->activePCIHostdevs,
   mgr->inactivePCIHostdevs) < 0)
@@ -608,6 +636,9 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci, actual;
 
+/* We need to look up the actual device and set the information
+ * there because 'pci' only contain address information and will
+ * be released at the end of the function */
 pci = virPCIDeviceListGet(pcidevs, i);
 actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci);
 
@@ -775,6 +806,10 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
 virPCIDevicePtr actual = NULL;
 
+/* We need to look up the actual device, which is the one containing
+ * information such as by which domain and driver it is used. As a
+ * side effect, by looking it up we can also tell whether it was
+ * really active in the first place */
 actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci);
 if (actual) {
 const char *actual_drvname;
@@ -830,6 +865,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
 
+/* We can avoid looking up the actual device here, because performing
+ * a PCI reset on a device doesn't require any information other than
+ * the address, which 'pci' already contains */
 VIR_DEBUG("Resetting PCI device %s", virPCIDeviceGetName(pci));
 if (virPCIDeviceReset(pci, mgr->activePCIHostdevs,
   mgr->inactivePCIHostdevs) < 0) {
-- 
2.5.0

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

[libvirt] [PATCH 04/24] tests: hostdev: Use size_t for count variables

[Andrea Bolognani]

virPCIDeviceListCount()'s return type is size_t, so variables that
store its return value should be of that type.
---
 tests/virhostdevtest.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 77b119b..c1321b1 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -39,10 +39,10 @@ VIR_LOG_INIT("tests.hostdevtest");
 
 # define CHECK_LIST_COUNT(list, cnt)\
 do {\
-int count;  \
+size_t count;   \
 if ((count = virPCIDeviceListCount(list)) != cnt) { \
 virReportError(VIR_ERR_INTERNAL_ERROR,  \
-   "Unexpected count of items in " #list ": %d, "   \
+   "Unexpected count of items in " #list ": %zu, "  \
"expecting %zu", count, (size_t) cnt);   \
 goto cleanup;   \
 }   \
@@ -163,8 +163,7 @@ static int
 testVirHostdevPreparePCIHostdevs_unmanaged(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t i;
-int active_count, inactive_count;
+size_t active_count, inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++)
  hostdevs[i]->managed = false;
@@ -222,8 +221,7 @@ static int
 testVirHostdevReAttachPCIHostdevs_unmanaged(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t i;
-int active_count, inactive_count;
+size_t active_count, inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != false) {
@@ -256,8 +254,7 @@ static int
 testVirHostdevPreparePCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t i;
-int active_count;
+size_t active_count, i;
 
 for (i = 0; i < nhostdevs; i++)
 hostdevs[i]->managed = true;
@@ -302,8 +299,7 @@ static int
 testVirHostdevReAttachPCIHostdevs_managed(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t i;
-int active_count;
+size_t active_count, i;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != true) {
@@ -334,8 +330,7 @@ static int
 testVirHostdevDetachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t i;
-int inactive_count;
+size_t inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++) {
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
@@ -371,8 +366,7 @@ static int
 testVirHostdevReAttachPCINodeDevice(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-size_t i;
-int inactive_count;
+size_t inactive_count, i;
 
 for (i = 0; i < nhostdevs; i++) {
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
@@ -392,7 +386,7 @@ static int
 testVirHostdevUpdateActivePCIHostdevs(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-int active_count;
+size_t active_count;
 
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
 
-- 
2.5.0

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

[libvirt] [PATCH 07/24] hostdev: Make comments easier to change later

[Andrea Bolognani]

Replace the term "loop" with the more generic "step". This allows us
to be more flexible and eg. have a step that consists in a single
function call.

Don't include the number of steps in the first comment of the
function, so that we can add or remove steps without having to worry
about keeping that comment in sync.

For the same reason, remove the summary contained in that comment.

Clean up some weird vertical spacing while we're at it.
---
 src/util/virhostdev.c | 58 +++
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 2db0da0..adc4eec 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -546,18 +546,18 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs)))
 goto cleanup;
 
-/* We have to use 9 loops here. *All* devices must
- * be detached before we reset any of them, because
- * in some cases you have to reset the whole PCI,
- * which impacts all devices on it. Also, all devices
- * must be reset before being marked as active.
- */
-
-/* Loop 1: validate that non-managed device isn't in use, eg
+/* Detaching devices from the host involves several steps; each
+ * of them is described at length below.
+ *
+ * All devices must be detached before we reset any of them,
+ * because in some cases you have to reset the whole PCI, which
+ * impacts all devices on it. Also, all devices must be reset
+ * before being marked as active */
+
+/* Step 1: validate that non-managed device isn't in use, eg
  * by checking that device is either un-bound, or bound
  * to pci-stub.ko
  */
-
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
@@ -588,7 +588,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 }
 }
 
-/* Loop 2: detach managed devices (i.e. bind to appropriate stub driver) */
+/* Step 2: detach managed devices (i.e. bind to appropriate stub driver) */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
@@ -608,7 +608,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 /* At this point, all devices are attached to the stub driver and have
  * been marked as inactive */
 
-/* Loop 3: Now that all the PCI hostdevs have been detached, we
+/* Step 3: Now that all the PCI hostdevs have been detached, we
  * can safely reset them */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
@@ -619,7 +619,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 goto reattachdevs;
 }
 
-/* Loop 4: For SRIOV network devices, Now that we have detached the
+/* Step 4: For SRIOV network devices, Now that we have detached the
  * the network device, set the netdev config */
 for (i = 0; i < nhostdevs; i++) {
  virDomainHostdevDefPtr hostdev = hostdevs[i];
@@ -632,7 +632,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
  last_processed_hostdev_vf = i;
 }
 
-/* Loop 5: Now mark all the devices as active */
+/* Step 5: Now mark all the devices as active */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
@@ -642,7 +642,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 goto inactivedevs;
 }
 
-/* Loop 6: Now remove the devices from inactive list. */
+/* Step 6: Now remove the devices from inactive list. */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
@@ -651,7 +651,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDeviceListDel(hostdev_mgr->inactivePCIHostdevs, dev);
 }
 
-/* Loop 7: Now set the used_by_domain of the device in
+/* Step 7: Now set the used_by_domain of the device in
  * activePCIHostdevs as domain name.
  */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
@@ -666,7 +666,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDeviceSetUsedBy(activeDev, drv_name, dom_name);
 }
 
-/* Loop 8: Now set the original states for hostdev def */
+/* Step 8: Now set the original states for hostdev def */
 for (i = 0; i < nhostdevs; i++) {
 virPCIDevicePtr dev;
 virPCIDevicePtr pcidev;
@@ -681,10 +681,9 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
   

[libvirt] [PATCH 18/24] hostdev: Use consistent variable names

[Andrea Bolognani]

This is not just a cosmetic change: the name of the variable now
gives a hint about what it is supposed to be used for.
---
 src/util/virhostdev.c | 234 +-
 1 file changed, 117 insertions(+), 117 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index a7fb8b1..10d1c1a 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -62,33 +62,33 @@ struct virHostdevIsPCINodeDeviceUsedData {
 
 static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void 
*opaque)
 {
-virPCIDevicePtr other;
+virPCIDevicePtr actual;
 int ret = -1;
 struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
 
-other = virPCIDeviceListFindByIDs(helperData->mgr->activePCIHostdevs,
-  devAddr->domain, devAddr->bus,
-  devAddr->slot, devAddr->function);
-if (other) {
-const char *other_drvname = NULL;
-const char *other_domname = NULL;
-virPCIDeviceGetUsedBy(other, _drvname, _domname);
+actual = virPCIDeviceListFindByIDs(helperData->mgr->activePCIHostdevs,
+   devAddr->domain, devAddr->bus,
+   devAddr->slot, devAddr->function);
+if (actual) {
+const char *actual_drvname = NULL;
+const char *actual_domname = NULL;
+virPCIDeviceGetUsedBy(actual, _drvname, _domname);
 
 if (helperData->usesVFIO &&
-(other_domname && helperData->domainName) &&
-(STREQ(other_domname, helperData->domainName)))
+(actual_domname && helperData->domainName) &&
+(STREQ(actual_domname, helperData->domainName)))
 goto iommu_owner;
 
-if (other_drvname && other_domname)
+if (actual_drvname && actual_domname)
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is in use by "
  "driver %s, domain %s"),
-   virPCIDeviceGetName(other),
-   other_drvname, other_domname);
+   virPCIDeviceGetName(actual),
+   actual_drvname, actual_domname);
 else
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is in use"),
-   virPCIDeviceGetName(other));
+   virPCIDeviceGetName(actual));
 goto cleanup;
 }
  iommu_owner:
@@ -203,45 +203,45 @@ virHostdevManagerGetDefault(void)
 static virPCIDeviceListPtr
 virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
 {
-virPCIDeviceListPtr list;
+virPCIDeviceListPtr pcidevs;
 size_t i;
 
-if (!(list = virPCIDeviceListNew()))
+if (!(pcidevs = virPCIDeviceListNew()))
 return NULL;
 
 for (i = 0; i < nhostdevs; i++) {
 virDomainHostdevDefPtr hostdev = hostdevs[i];
 virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
-virPCIDevicePtr dev;
+virPCIDevicePtr pci;
 
 if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
 continue;
 if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
 continue;
 
-dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
+pci = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
   pcisrc->addr.slot, pcisrc->addr.function);
-if (!dev) {
-virObjectUnref(list);
+if (!pci) {
+virObjectUnref(pcidevs);
 return NULL;
 }
-if (virPCIDeviceListAdd(list, dev) < 0) {
-virPCIDeviceFree(dev);
-virObjectUnref(list);
+if (virPCIDeviceListAdd(pcidevs, pci) < 0) {
+virPCIDeviceFree(pci);
+virObjectUnref(pcidevs);
 return NULL;
 }
 
-virPCIDeviceSetManaged(dev, hostdev->managed);
+virPCIDeviceSetManaged(pci, hostdev->managed);
 
 if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
-virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_VFIO);
+virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
 else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
-virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_XEN);
+virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN);
 else
-virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_KVM);
+virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM);
 }
 
-return list;
+return pcidevs;
 }
 
 
@@ -511,15 +511,15 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  * to pci-stub.ko
  */
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
-virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
+

[libvirt] [PATCH 09/24] hostdev: Fix indentation

[Andrea Bolognani]

---
 src/util/virhostdev.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 5be61cd..48a44bc 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -833,9 +833,10 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr 
hostdev_mgr,
 virPCIDeviceGetUsedBy(activeDev, _drvname, _domname);
 if (STRNEQ_NULLABLE(drv_name, usedby_drvname) ||
 STRNEQ_NULLABLE(dom_name, usedby_domname)) {
-virPCIDeviceListDel(pcidevs, dev);
-continue;
-}
+
+virPCIDeviceListDel(pcidevs, dev);
+continue;
+}
 }
 
 VIR_DEBUG("Removing PCI device %s from active list",
-- 
2.5.0

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

[libvirt] [PATCH 24/24] tests: hostdev: Add more tests

[Andrea Bolognani]

Ensure the code behaves properly even for situations that were not
being considered before, such as simply detaching devices from the
host without attaching them to a guest and attaching devices as
managed even though they had already been manually detached from
the host.
---
 tests/virhostdevtest.c | 84 ++
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index c0ab044..617e86c 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -253,7 +253,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(void)
 }
 
 static int
-testVirHostdevPreparePCIHostdevs_managed(void)
+testVirHostdevPreparePCIHostdevs_managed(bool mixed)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -270,7 +270,13 @@ testVirHostdevPreparePCIHostdevs_managed(void)
  hostdevs, nhostdevs, 0) < 0)
 goto cleanup;
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count + nhostdevs);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
+/* If testing a mixed roundtrip, devices are already in the inactive list
+ * before we start and are removed from it as soon as we attach them to
+ * the guest */
+if (mixed)
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count - nhostdevs);
+else
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 /* Test conflict */
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
@@ -304,7 +310,7 @@ testVirHostdevPreparePCIHostdevs_managed(void)
 }
 
 static int
-testVirHostdevReAttachPCIHostdevs_managed(void)
+testVirHostdevReAttachPCIHostdevs_managed(bool mixed)
 {
 int ret = -1;
 size_t active_count, inactive_count, i;
@@ -328,7 +334,12 @@ testVirHostdevReAttachPCIHostdevs_managed(void)
 virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
   hostdevs, nhostdevs, NULL);
 CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count - nhostdevs);
-CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
+/* If testing a mixed roundtrip, devices are added back to the inactive
+ * list as soon as we detach from the guest */
+if (mixed)
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count + nhostdevs);
+else
+CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
 
 ret = 0;
 
@@ -432,6 +443,31 @@ testVirHostdevUpdateActivePCIHostdevs(void)
 }
 
 /**
+ * testVirHostdevRoundtripNoGuest:
+ * @opaque: unused
+ *
+ * Perform a roundtrip without ever assigning devices to the guest.
+ *
+ *   1. Detach devices from the host
+ *   2. Reattach devices to the host
+ */
+static int
+testVirHostdevRoundtripNoGuest(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (testVirHostdevDetachPCINodeDevice() < 0)
+goto out;
+if (testVirHostdevReAttachPCINodeDevice() < 0)
+goto out;
+
+ret = 0;
+
+ out:
+return ret;
+}
+
+/**
  * testVirHostdevRoundtripUnmanaged:
  * @opaque: unused
  *
@@ -479,9 +515,9 @@ testVirHostdevRoundtripManaged(const void *opaque 
ATTRIBUTE_UNUSED)
 int ret = -1;
 
 if (virHostdevHostSupportsPassthroughKVM()) {
-if (testVirHostdevPreparePCIHostdevs_managed() < 0)
+if (testVirHostdevPreparePCIHostdevs_managed(false) < 0)
 goto out;
-if (testVirHostdevReAttachPCIHostdevs_managed() < 0)
+if (testVirHostdevReAttachPCIHostdevs_managed(false) < 0)
 goto out;
 }
 
@@ -492,6 +528,40 @@ testVirHostdevRoundtripManaged(const void *opaque 
ATTRIBUTE_UNUSED)
 }
 
 /**
+ * testVirHostdevRoundtripMixed:
+ * @opaque: unused
+ *
+ * Perform a roundtrip with managed devices but manually detach the devices
+ * from the host first.
+ *
+ *   1. Detach devices from the host
+ *   2. Attach devices to the guest as managed
+ *   3. Detach devices from the guest as managed
+ *   4. Reattach devices to the host
+ */
+static int
+testVirHostdevRoundtripMixed(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+
+if (testVirHostdevDetachPCINodeDevice() < 0)
+goto out;
+if (virHostdevHostSupportsPassthroughKVM()) {
+if (testVirHostdevPreparePCIHostdevs_managed(true) < 0)
+goto out;
+if (testVirHostdevReAttachPCIHostdevs_managed(true) < 0)
+goto out;
+}
+if (testVirHostdevReAttachPCINodeDevice() < 0)
+goto out;
+
+ret = 0;
+
+ out:
+return ret;
+}
+
+/**
  * testVirHostdevOther:
  * @opaque: unused
  *
@@ -546,8 +616,10 @@ mymain(void)
 if (myInit() < 0)
 fprintf(stderr, "Init data structures failed.");
 
+DO_TEST(testVirHostdevRoundtripNoGuest);
 DO_TEST(testVirHostdevRoundtripUnmanaged);
 DO_TEST(testVirHostdevRoundtripManaged);
+DO_TEST(testVirHostdevRoundtripMixed);
 DO_TEST(testVirHostdevOther);
 
 myCleanup();
-- 
2.5.0

--
libvir-list mailing 

Re: [libvirt] GSoC 2016

[Peter Krempa]

On Mon, Mar 07, 2016 at 16:50:41 +0100, Martin Kletzander wrote:
> Hi,
> 
> it's good to hear that you are interested.  I'll give you the same hints
> as I give others for now.  But before that, have you had a chance of
> looking at the ideas page of libvirt?  Is any one of those ideas
> appealing to you or do you have your own
> 
> As for the ideas, look at our documentation, clone the repository [1],
> try building it [2], running tests etc., look at the contributor
> guidelines [2] and generally browse the site to know about some basics.
> You can also browse the code on the web [4].  Then you can go through
> the code and see how it all works; or the parts that interest you the
> most, for now.
> 
> Be sure to fill in the application before the deadline, the time window
> is pretty short this year.

You should add the above to the wiki page with libvirt's GSOC info. It
will be useful.

Additionally google's page contains link only to our web page and the
email address. There's no link to the wiki including the project idea
list.

Michal, could you ask to add the wiki page link to the GSoC page [1], so
that we don't get as many redundant questions?

[1] https://summerofcode.withgoogle.com/organizations/5982501017223168/

Peter



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

Re: [libvirt] [PATCH 01/10] secret: Move virSecretObj to secret_conf.h

[Peter Krempa]

On Wed, Mar 02, 2016 at 13:54:58 -0500, John Ferlan wrote:
> To support being able to create a hashed secrets list, move the
> virSecretObj to secret_conf.h so that secret_conf.c can at least find
> the definition.

I don't think this is necessary. You still can create accessors and have
the actual implementation of the struct private in the .c file. That way
it's guaranteed that nobody will touch the fields directly accross the
source files.

Peter


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

Re: [libvirt] [PATCH 02/10] Add secretObjFromSecret

[Peter Krempa]

On Wed, Mar 02, 2016 at 13:54:59 -0500, John Ferlan wrote:
> Signed-off-by: John Ferlan 
> ---
>  src/secret/secret_driver.c | 45 +
>  1 file changed, 21 insertions(+), 24 deletions(-)

ACK


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

Re: [libvirt] GSoC 2016

[Martin Kletzander]

On Mon, Mar 07, 2016 at 12:30:40AM +0500, Tahir Ramzan wrote:

Respected Sir,

I am a MS CS scholar of Virtual University of Pakistan, I want to
participate in GSoC 2016 for Libvirt. Data Science, Networks, Information
security, digital forensics and ethical hacking are my core areas of
interest.

Currently, I am working on a research project on live forensics of GPU and
volatile memories like RAMs and Caches.

I am looking forward your guidance to start my contribution for Libvirt,
thanks in anticipation.



Hi,

it's good to hear that you are interested.  I'll give you the same hints
as I give others for now.  But before that, have you had a chance of
looking at the ideas page of libvirt?  Is any one of those ideas
appealing to you or do you have your own

As for the ideas, look at our documentation, clone the repository [1],
try building it [2], running tests etc., look at the contributor
guidelines [2] and generally browse the site to know about some basics.
You can also browse the code on the web [4].  Then you can go through
the code and see how it all works; or the parts that interest you the
most, for now.

Be sure to fill in the application before the deadline, the time window
is pretty short this year.

Have a nice day,
Martin

[1] git://libvirt.org/libvirt.git
[2] https://libvirt.org/compiling.html
[3] https://libvirt.org/hacking.html
[4] https://libvirt.org/git/?p=libvirt.git;a=summary


Regards
Tahir Ramzan



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


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

Re: [libvirt] ruby-libvirt mixed storage migration

[Tim Förster - Hetzner Online AG]

Hi,

rebuilding the gem is not quite easy. I am not able to find the actual gemspec. 
There are also some
missing features, which are available though virsh. Such as 'virsh domblklist' 
to find out the
actual registered block targets or rate limiting with iotune the burst iops.

tim

Am 07.03.2016 um 15:52 schrieb Chris Lalancette:
> Hi there,
> 
> On Thu, Mar 3, 2016 at 11:10 AM, Tim Förster - Hetzner Online AG 
>  > wrote:
> 
> Heyho guys,
> 
> is there any reason why mixed storage migration is restriced. I am unable 
> to serve parameter
> "migrate_disks" through #migrate_to_uri3. It looks like there is a 
> missing macro here.
> 
> http://libvirt.org/git/?p=ruby-libvirt.git;a=blob;f=ext/libvirt/domain.c#l3858.
>  Could you please
> verify this?
> 
> 
> This is probably one of those enums I just haven't gotten to yet.  I can look 
> at implementing it for
> the next release.  Note, though, that all of these fields are integers, so as 
> a temporary workaround
> you should be able to just use the correct number.
> 
> Chris



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

[libvirt] [libvirt-php 0/3] small improvements

[Vasiliy Tolstov]

Add some more libvirt functions support to binding.
Also add some checks to prevent malloc SIZE_MAX memory
in case of errors

Vasiliy Tolstov (3):
  add some checks to prevent overflow
  add block_commit support and needed const
  add libvirt_domain_block_job_info

 src/libvirt-php.c | 152 --
 src/libvirt-php.h |   2 +
 2 files changed, 138 insertions(+), 16 deletions(-)

--
2.7.0

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

Re: [libvirt] [PATCH v2 6/7] Introduce job completed event

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:32 +0100, Jiri Denemark wrote:
> The VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event will be triggered once a job
> (such as migration) finishes and it will contain statistics for the job
> as one would get by calling virDomainGetJobStats. Thanks to this event
> it is now possible to get statistics of a completed migration of a
> transient domain on the source host.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK


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

Re: [libvirt] [PATCH v2 7/7] qemu: Add support for job completed event

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:33 +0100, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---

ACK


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

[libvirt] [libvirt-php 3/3] add libvirt_domain_block_job_info

[Vasiliy Tolstov]

Signed-off-by: Vasiliy Tolstov 
Signed-off-by: Yuriy Gromak 
---
 src/libvirt-php.c | 36 
 src/libvirt-php.h |  1 +
 2 files changed, 37 insertions(+)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 8e03d86a811a..1bc837b73e74 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -143,6 +143,7 @@ static zend_function_entry libvirt_functions[] = {
 PHP_FE(libvirt_domain_block_resize,NULL)
 PHP_FE(libvirt_domain_block_job_abort,NULL)
 PHP_FE(libvirt_domain_block_job_set_speed,NULL)
+PHP_FE(libvirt_domain_block_job_info,NULL)
 PHP_FE(libvirt_domain_interface_stats,NULL)
 PHP_FE(libvirt_domain_get_connect, NULL)
 PHP_FE(libvirt_domain_migrate, NULL)
@@ -1371,6 +1372,8 @@ PHP_MINIT_FUNCTION(libvirt)
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT",  
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, CONST_CS | CONST_PERSISTENT);
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES",
VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
 
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES",
VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
+
 
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN", 
VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN, CONST_CS | CONST_PERSISTENT);
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_PULL", 
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL, CONST_CS | CONST_PERSISTENT);
@@ -6145,6 +6148,39 @@ PHP_FUNCTION(libvirt_domain_block_commit)
 RETURN_TRUE;
 }
 
+
+/*
+ * Function name:   libvirt_domain_block_job_info
+ * Since version:   0.5.2(-1)
+ * Description: Function is used to request block job information for 
the given disk
+ * Arguments:   @dom [resource]: libvirt domain resource, e.g. from 
libvirt_domain_lookup_by_*()
+ *  @disk [string]: path to the block device, or device 
shorthand
+ *  @flags [int]: bitwise-OR of VIR_DOMAIN_BLOCK_COMMIT_*
+ * Returns: Array with status virDomainGetBlockJobInfo and 
blockjob information. 
+*/
+PHP_FUNCTION(libvirt_domain_block_job_info)
+{
+  php_libvirt_domain *domain=NULL;
+  zval *zdomain;
+  int retval;
+  char *disk;
+  int disk_len;
+  long flags = 0;
+  virDomainBlockJobInfo info;
+  
+  GET_DOMAIN_FROM_ARGS("rs|l",, , _len, );
+  
+  retval=virDomainGetBlockJobInfo(domain->domain, disk, , flags);
+  
+  array_init(return_value);
+  add_assoc_long(return_value, "status", (int)retval);
+  add_assoc_long(return_value, "type", (int)info.type);
+  add_assoc_long(return_value, "bandwidth", (unsigned 
long)info.bandwidth);
+  add_assoc_long(return_value, "cur", (unsigned long long)info.cur);
+  add_assoc_long(return_value, "end", (unsigned long long)info.end);
+}
+  
+
 /*
  * Function name:   libvirt_domain_block_job_abort
  * Since version:   0.5.1(-1)
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index cb06c2d0b8a7..a45b4bd62c64 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -408,6 +408,7 @@ PHP_FUNCTION(libvirt_domain_block_stats);
 PHP_FUNCTION(libvirt_domain_block_resize);
 PHP_FUNCTION(libvirt_domain_block_job_abort);
 PHP_FUNCTION(libvirt_domain_block_job_set_speed);
+PHP_FUNCTION(libvirt_domain_block_job_info);
 PHP_FUNCTION(libvirt_domain_interface_stats);
 PHP_FUNCTION(libvirt_domain_get_connect);
 PHP_FUNCTION(libvirt_domain_migrate);
-- 
2.7.0

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

[libvirt] [libvirt-php 1/3] add some checks to prevent overflow

[Vasiliy Tolstov]

Signed-off-by: Vasiliy Tolstov 
---
 src/libvirt-php.c | 50 +++---
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 57e1b83ffa2a..4034458e0e4f 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -6876,7 +6876,9 @@ PHP_FUNCTION(libvirt_storagepool_list_volumes)
 
 GET_STORAGEPOOL_FROM_ARGS("r",);
 
-expectedcount=virStoragePoolNumOfVolumes (pool->pool);
+if ((expectedcount=virStoragePoolNumOfVolumes (pool->pool)) < 0)
+RETURN_FALSE;
+
 DPRINTF("%s: virStoragePoolNumOfVolumes(%p) returned %d\n", PHPFUNC, 
pool->pool, expectedcount);
 names=(char **)emalloc(expectedcount*sizeof(char *));
 
@@ -7741,7 +7743,8 @@ PHP_FUNCTION(libvirt_list_storagepools)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfStoragePools(conn->conn);
+if ((expectedcount=virConnectNumOfStoragePools(conn->conn)) < 0)
+RETURN_FALSE;
 
 names=(char **)emalloc(expectedcount*sizeof(char *));
 count=virConnectListStoragePools(conn->conn,names,expectedcount);
@@ -7761,7 +7764,8 @@ PHP_FUNCTION(libvirt_list_storagepools)
 efree(names);
 
 
-expectedcount = virConnectNumOfDefinedStoragePools (conn->conn);
+if ((expectedcount = virConnectNumOfDefinedStoragePools (conn->conn)) < 0)
+RETURN_FALSE;
 names = (char **)emalloc (expectedcount * sizeof(char *));
 count = virConnectListDefinedStoragePools (conn->conn, names, 
expectedcount);
 if ((count != expectedcount) || (count < 0))
@@ -7796,7 +7800,8 @@ PHP_FUNCTION(libvirt_list_active_storagepools)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfStoragePools(conn->conn);
+if ((expectedcount=virConnectNumOfStoragePools(conn->conn)) < 0)
+RETURN_FALSE;
 
 names=(char **)emalloc(expectedcount*sizeof(char *));
 count=virConnectListStoragePools(conn->conn,names,expectedcount);
@@ -7833,7 +7838,9 @@ PHP_FUNCTION(libvirt_list_inactive_storagepools)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount = virConnectNumOfDefinedStoragePools (conn->conn);
+if ((expectedcount = virConnectNumOfDefinedStoragePools (conn->conn)) < 0)
+RETURN_FALSE;
+
 names= (char **)emalloc (expectedcount * sizeof(char *));
 count = virConnectListDefinedStoragePools (conn->conn, names, 
expectedcount);
 if ((count != expectedcount) || (count < 0))
@@ -7872,7 +7879,9 @@ PHP_FUNCTION(libvirt_list_domains)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfDomains (conn->conn);
+if ((expectedcount=virConnectNumOfDomains (conn->conn)) < 0)
+RETURN_FALSE;
+
 DPRINTF("%s: Found %d domains\n", PHPFUNC, expectedcount);
 
 ids=(int *)emalloc(sizeof(int)*expectedcount);
@@ -7953,7 +7962,8 @@ PHP_FUNCTION(libvirt_list_domain_resources)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfDomains (conn->conn);
+if ((expectedcount=virConnectNumOfDomains (conn->conn)) < 0)
+RETURN_FALSE;
 
 ids=(int *)emalloc(sizeof(int)*expectedcount);
 count=virConnectListDomains (conn->conn,ids,expectedcount);
@@ -7981,7 +7991,9 @@ PHP_FUNCTION(libvirt_list_domain_resources)
 }
 efree(ids);
 
-expectedcount=virConnectNumOfDefinedDomains (conn->conn);
+if ((expectedcount=virConnectNumOfDefinedDomains (conn->conn)) < 0)
+RETURN_FALSE;
+
 names=(char **)emalloc(expectedcount*sizeof(char *));
 count=virConnectListDefinedDomains (conn->conn,names,expectedcount);
 if ((count != expectedcount) || (count<0))
@@ -8027,7 +8039,8 @@ PHP_FUNCTION(libvirt_list_active_domain_ids)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfDomains (conn->conn);
+if ((expectedcount=virConnectNumOfDomains (conn->conn)) < 0)
+RETURN_FALSE;
 
 ids=(int *)emalloc(sizeof(int)*expectedcount);
 count=virConnectListDomains (conn->conn,ids,expectedcount);
@@ -8064,7 +8077,8 @@ PHP_FUNCTION(libvirt_list_active_domains)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfDomains (conn->conn);
+if ((expectedcount=virConnectNumOfDomains (conn->conn)) <)
+RETURN_FALSE;
 
 ids=(int *)emalloc(sizeof(int)*expectedcount);
 count=virConnectListDomains (conn->conn,ids,expectedcount);
@@ -8115,7 +8129,8 @@ PHP_FUNCTION(libvirt_list_inactive_domains)
 
 GET_CONNECTION_FROM_ARGS("r",);
 
-expectedcount=virConnectNumOfDefinedDomains (conn->conn);
+if ((expectedcount=virConnectNumOfDefinedDomains (conn->conn)) < 0)
+RETURN_FALSE;
 
 names=(char **)emalloc(expectedcount*sizeof(char *));
 count=virConnectListDefinedDomains (conn->conn,names,expectedcount);
@@ -8156,7 +8171,9 @@ PHP_FUNCTION(libvirt_list_networks)
 
 array_init(return_value);
 if (flags & VIR_NETWORKS_ACTIVE) {
-

[libvirt] [libvirt-php 2/3] add block_commit support and needed const

[Vasiliy Tolstov]

Signed-off-by: Vasiliy Tolstov 
---
 src/libvirt-php.c | 66 ++-
 src/libvirt-php.h |  1 +
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 4034458e0e4f..8e03d86a811a 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -138,6 +138,7 @@ static zend_function_entry libvirt_functions[] = {
 PHP_FE(libvirt_domain_set_memory,NULL)
 PHP_FE(libvirt_domain_set_max_memory,NULL)
 PHP_FE(libvirt_domain_set_memory_flags,NULL)
+PHP_FE(libvirt_domain_block_commit, NULL)
 PHP_FE(libvirt_domain_block_stats,NULL)
 PHP_FE(libvirt_domain_block_resize,NULL)
 PHP_FE(libvirt_domain_block_job_abort,NULL)
@@ -1355,11 +1356,40 @@ PHP_MINIT_FUNCTION(libvirt)
 /* Job was aborted but it's not cleanup up yet */
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_JOB_CANCELLED",  5, CONST_CS | 
CONST_PERSISTENT);
 
+
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COMMIT_SHALLOW", 
VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COMMIT_DELETE", 
VIR_DOMAIN_BLOCK_COMMIT_DELETE, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COMMIT_ACTIVE", 
VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COMMIT_RELATIVE", 
VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES", 
VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
+
+
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COPY_SHALLOW", 
VIR_DOMAIN_BLOCK_COPY_SHALLOW, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_COPY_REUSE_EXT", 
VIR_DOMAIN_BLOCK_COPY_REUSE_EXT, CONST_CS | CONST_PERSISTENT);
+
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC",  
VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC, CONST_CS | CONST_PERSISTENT);
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT",  
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, CONST_CS | CONST_PERSISTENT);
-
 REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES",
VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
 
+
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN", 
VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_PULL", 
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_COPY", 
VIR_DOMAIN_BLOCK_JOB_TYPE_COPY, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT", 
VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT", 
VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT, CONST_CS | CONST_PERSISTENT);
+
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES", 
VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
+
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_SHALLOW", 
VIR_DOMAIN_BLOCK_REBASE_SHALLOW, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT", 
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_COPY_RAW", 
VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_COPY", 
VIR_DOMAIN_BLOCK_REBASE_COPY, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_RELATIVE", 
VIR_DOMAIN_BLOCK_REBASE_RELATIVE, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_COPY_DEV", 
VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, CONST_CS | CONST_PERSISTENT);
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES", 
VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
+
+REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_RESIZE_BYTES", 
VIR_DOMAIN_BLOCK_RESIZE_BYTES, CONST_CS | CONST_PERSISTENT);
+
 /* Migration constants */
 REGISTER_LONG_CONSTANT("VIR_MIGRATE_LIVE",1, CONST_CS | 
CONST_PERSISTENT);
 /* direct source -> dest host control channel Note the less-common 
spelling that we're stuck with: */
@@ -6082,6 +6112,40 @@ PHP_FUNCTION(libvirt_domain_block_resize)
 }
 
 /*
+ * Function name:   libvirt_domain_block_commit
+ * Since version:   0.5.2(-1)
+ * Description: Function is used to commit block job
+ * Arguments:   @res [resource]: libvirt domain resource, e.g. from 
libvirt_domain_lookup_by_*()
+ *  @disk [string]: path to the block device, or device 
shorthand
+ *  @base [string]: path to backing file to merge into, or 
device shorthand, or NULL for default
+ *  @top [string]: path to file within backing chain that 
contains data to be merged, or device shorthand, or NULL to merge all 

Re: [libvirt] ruby-libvirt mixed storage migration

[Chris Lalancette]

Hi there,

On Thu, Mar 3, 2016 at 11:10 AM, Tim Förster - Hetzner Online AG <
tim.foers...@hetzner.de> wrote:

> Heyho guys,
>
> is there any reason why mixed storage migration is restriced. I am unable
> to serve parameter
> "migrate_disks" through #migrate_to_uri3. It looks like there is a missing
> macro here.
>
> http://libvirt.org/git/?p=ruby-libvirt.git;a=blob;f=ext/libvirt/domain.c#l3858.
> Could you please
> verify this?
>

This is probably one of those enums I just haven't gotten to yet.  I can
look at implementing it for the next release.  Note, though, that all of
these fields are integers, so as a temporary workaround you should be able
to just use the correct number.

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

Re: [libvirt] "opertaions" typo on wiki.

[Daniel P. Berrange]

On Sun, Mar 06, 2016 at 03:58:24AM -0800, Dennis wrote:
> Hi,
> 
> I tried to edit  http://wiki.libvirt.org/page/Main_Page to fix a typo of
> the word 'operations,' (see email subject) but could not figure out how to
> create an account.
> 
> 
> I read on the contact page that if I am not subscribed to the mailing list
> I am mailing this small insignificant task won't be emailed to a large
> group of people and will be moderated appropriately.

I've fixed the typo - thanks for pointing it out.  We had to disable
account creation due to spammers. If you still want an account let me
know and I can create one.


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] [libvirt-glib] gconfig: Fix next API version number

[Christophe Fergeau]

On Fri, Mar 04, 2016 at 02:54:20PM +0100, Fabiano Fidêncio wrote:
> Commit 01e01f2 introduced a wrong next API version number in
> libvirt-gconfig.sym

libvirt-glib 0.2.3 was released in december, I believe we try to align
API versioning with tarball versions, in which case 0.2.4 is correct
here.

Christophe

> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  libvirt-gconfig/libvirt-gconfig.sym | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
> b/libvirt-gconfig/libvirt-gconfig.sym
> index f11f97a..5bc2e24 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -733,7 +733,7 @@ global:
>   gvir_config_domain_video_set_vgamem;
>  } LIBVIRT_GCONFIG_0.2.1;
>  
> -LIBVIRT_GCONFIG_0.2.4 {
> +LIBVIRT_GCONFIG_0.2.3 {
>   gvir_config_domain_graphics_spice_set_gl;
>   gvir_config_domain_video_set_accel3d;
>  } LIBVIRT_GCONFIG_0.2.2;
> -- 
> 2.5.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

[libvirt] [PATCH] qemu: ignore EACCES error on hot unplug of PCI devices with VFIO

[Kothapally Madhu Pavan]

When the hostdev managed=true, the VFIO devices are reattached
to the host driver before coming to qemuTeardownHostdevCgroup().
So, the /dev/vfio/ path would not exist. We ignore the
EACCES error here.

Signed-off-by: Kothapally Madhu Pavan 
---
 src/qemu/qemu_cgroup.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index c76d585..2850346 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -427,7 +427,7 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
 
 VIR_DEBUG("Cgroup deny %s for PCI device assignment", path);
 rv = virCgroupDenyDevicePath(priv->cgroup, path,
- VIR_CGROUP_DEVICE_RWM, false);
+ VIR_CGROUP_DEVICE_RWM, 
dev->managed);
 virDomainAuditCgroupPath(vm, priv->cgroup,
  "deny", path, "rwm", rv == 0);
 if (rv < 0)

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

Re: [libvirt] [PATCH v2 5/7] qemu: Do not report completed stats until the job finishes

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:31 +0100, Jiri Denemark wrote:
> We would happily report and free statistics of a completed migration
> even before it actually completed (on the source host while migration is
> in the Finish phase).
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK


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

Re: [libvirt] [PATCH v2 4/7] qemu: Fix a race when computing migration downtime

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:30 +0100, Jiri Denemark wrote:
> Computing a total downtime during a migration requires us to store a
> time stamp when guest CPUs get stopped. The value (and all other
> statistics) is then transferred to the destination to compute the
> downtime. Because the stopped time stamp is stored by a STOP event
> handler while the statistics which will be sent over to the destination
> are copied synchronously within qemuMigrationWaitForCompletion.
> 
> Depending on the timing of STOP and MIGRATION events, we may end up
> copying (and transferring) statistics without the stopped time stamp
> set. Let's make sure we always use the correct time stamp.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1282744
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK


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

Re: [libvirt] [PATCH v2 3/7] qemu: Don't explicitly stop CPUs after migration

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:29 +0100, Jiri Denemark wrote:
> With a very old QEMU which doesn't support events we need to explicitly
> call qemuMigrationSetOffline at the end of migration to update our
> internal state. On the other hand, if we talk to QEMU using QMP, we
> should just wait for the STOP event and let the event handler update the
> state and trigger a libvirt event.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK


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

Re: [libvirt] [PATCH v2 2/7] qemu: Properly update completed migration stats

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:28 +0100, Jiri Denemark wrote:
> We should not overwrite all migration statistics on the source with the
> numbers sent by the destination since the source may have an updated
> view in some cases (such as post-copy migration). It's safer to update
> just the timing info we need to get from the destination and be prepared
> for the future. And we should only do all this after a successful
> migration.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK


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

[libvirt] [PATCH] Fix minor typos in messages

[Yuri Chornoivan]

Hi,

Attached is a patch to fix minor typos in libvirt messages. Thanks for  
fixing them in the repo.


By the way, libvirt template has not been updated for almost 1 year on  
Zanata. Are the translation moved into some new location?


Thanks in advance for your answers.

Best regards,
Yuri

0001-Fix-minor-typos.patch
Description: Binary data
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 1/7] qemu: Store completed stats at the very end of migration

[Peter Krempa]

On Mon, Mar 07, 2016 at 14:27:46 +0100, Peter Krempa wrote:
> On Tue, Mar 01, 2016 at 13:55:27 +0100, Jiri Denemark wrote:
> > Statistics for a completed migration only make sense if the migration
> > was successful. Let's don't store them in priv->job.completed until we

Let's not ... 

> > are sure it was a success.


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

Re: [libvirt] [PATCH v2 1/7] qemu: Store completed stats at the very end of migration

[Peter Krempa]

On Tue, Mar 01, 2016 at 13:55:27 +0100, Jiri Denemark wrote:
> Statistics for a completed migration only make sense if the migration
> was successful. Let's don't store them in priv->job.completed until we
> are sure it was a success.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 2:
> - no change
> 
>  src/qemu/qemu_migration.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)a

ACK


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

[libvirt] GSoC 2016

[Tahir Ramzan]

Respected Sir,

I am a MS CS scholar of Virtual University of Pakistan, I want to
participate in GSoC 2016 for Libvirt. Data Science, Networks, Information
security, digital forensics and ethical hacking are my core areas of
interest.

Currently, I am working on a research project on live forensics of GPU and
volatile memories like RAMs and Caches.

I am looking forward your guidance to start my contribution for Libvirt,
thanks in anticipation.

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

[libvirt] Ext transport mode libvirt

[nidhi d]

How do i create a new mode of communication between source and destination
using ext mode of transport?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] "opertaions" typo on wiki.

[Dennis]

Hi,

I tried to edit  http://wiki.libvirt.org/page/Main_Page to fix a typo of
the word 'operations,' (see email subject) but could not figure out how to
create an account.


I read on the contact page that if I am not subscribed to the mailing list
I am mailing this small insignificant task won't be emailed to a large
group of people and will be moderated appropriately.



Thanks,

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

Re: [libvirt] [PATCH 13/25] qemu: Introduce qemuBuildChannelsCommandLine

[Peter Krempa]

On Tue, Mar 01, 2016 at 07:43:10 -0500, John Ferlan wrote:
> 
> 
> On 02/18/2016 12:50 PM, John Ferlan wrote:
> > Add new function to manage adding the channel device options to the
> > command line removing that task from the mainline qemuBuildCommandLine.
> > 
> > Signed-off-by: John Ferlan 
> > ---
> >  src/qemu/qemu_command.c | 159 
> > ++--
> >  1 file changed, 87 insertions(+), 72 deletions(-)
> > 
> 
> I've merged my local branch with the latest upstream taking into account
> Martin's commit 'a89f05ba8d' with the following adjustment:
> 
> The rest of the changes merged fine. I can repost (in parts) if
> desired...  To make it less onerous to review such a large pile.

I'd suggest you push it to a repository where it can be fetched without
having to deal with conflicts while applying from email.


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

Re: [libvirt] [PATCH] locking: Use bit shift for flag values not constant values.

[Peter Krempa]

On Fri, Mar 04, 2016 at 09:39:35 -0500, John Ferlan wrote:
> So far it hasn't bitten us, but if the next value wasn't 4, then
> the logic used to check flag bits would have issues.
> 
> Signed-off-by: John Ferlan 
> ---

ACK, the second power could be mistaken with a sequential numbering.
After that it's again pretty obvious.

Peter


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

Re: [libvirt] [PATCH] _virtualboxCreateMachine: Avoid unbound stack

[Peter Krempa]

On Sat, Mar 05, 2016 at 14:05:31 +0100, Michal Privoznik wrote:
> If the stars are in the right position and you're building with

One would think that electron alignment in the material of your SSD that
creates the bits that finally align to the file creating the software on
your machine would cause this ... I'm not sure that star alignment has
to do much with it.

> VBox >= 4.2.0 it will happen that compiler thinks an array
> allocated on the stack may be unbound:

There's quite a substantial semantic difference between "unbound" and

> In file included from vbox/vbox_V4_2.c:13:0:
> vbox/vbox_tmpl.c: In function '_virtualboxCreateMachine':
> vbox/vbox_tmpl.c:2811:1: error: stack usage might be unbounded 
> [-Werror=stack-usage=]
 
... unbounded. I'd suggest you get your language straight. Plus one
other instance in the subhect.

>  _virtualboxCreateMachine(vboxGlobalData *data, virDomainDefPtr def, IMachine 
> **machine, char *uuidstr ATTRIBUTE_UNUSED)
>  ^
> 
> Well, given how the variable is declared, I had some hard time
> seeing it is actually bounded. Surprisingly compiler does not

Here it's correct!

> complain because of -Wframe-larger-than. This is because
> variable length arrays do not count into that warning.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/vbox/vbox_tmpl.c | 18 +++---
>  1 file changed, 7 insertions(+), 11 deletions(-)

ACK, the original code was a mess ...

Peter


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

Re: [libvirt] [PATCH] qemu: improve the error when try to undefine transient network

[Peter Krempa]

On Sun, Mar 06, 2016 at 18:54:21 +0800, Shanzhi Yu wrote:

I'll add the bugzilla link.

> Signed-off-by: Shanzhi Yu 
> ---
>  src/network/bridge_driver.c | 6 ++
>  1 file changed, 6 insertions(+)

ACK, pushed.


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

Re: [libvirt] [PATCH] qemu: rename: Forbid renaming domains with managed save image

[Jiri Denemark]

On Mon, Mar 07, 2016 at 10:08:12 +0100, Peter Krempa wrote:
> The code does not handle renaming of the save state file. In addition to
> that the resuming code would need to be tweaked to handle the name
> change since the XML is extracted from the save image. The easies option
> is to make the rename API even less useful by forbiding this.
> ---
>  src/qemu/qemu_driver.c | 6 ++
>  1 file changed, 6 insertions(+)

ACK

Jirka

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

[libvirt] [PATCH] qemu: rename: Forbid renaming domains with managed save image

[Peter Krempa]

The code does not handle renaming of the save state file. In addition to
that the resuming code would need to be tweaked to handle the name
change since the XML is extracted from the save image. The easies option
is to make the rename API even less useful by forbiding this.
---
 src/qemu/qemu_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 102fade..4020364 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19994,6 +19994,12 @@ static int qemuDomainRename(virDomainPtr dom,
 goto endjob;
 }

+if (vm->hasManagedSave) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("domain with a managed saved state can't be 
renamed"));
+goto endjob;
+}
+
 if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) {
 virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain has to be shutoff before renaming"));
-- 
2.6.2

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

[libvirt] [PATCH v3] Use correct pci addresses during interface-detach

[Nitesh Konkar]

From: Nitesh_Konkar 

---
 tools/virsh-domain.c | 103 +--
 1 file changed, 64 insertions(+), 39 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 979f115..a15ef40 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
 
-static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+static bool 
+vshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, 
vshControl *ctl, const vshCmd *cmd)
 {
-virDomainPtr dom = NULL;
 xmlDocPtr xml = NULL;
 xmlXPathObjectPtr obj = NULL;
 xmlXPathContextPtr ctxt = NULL;
 xmlNodePtr cur = NULL, matchNode = NULL;
-char *detach_xml = NULL;
 const char *mac = NULL, *type = NULL;
-char *doc = NULL;
+char *detach_xml = NULL;
+bool current = vshCommandOptBool(cmd, "current");
 char buf[64];
 int diff_mac;
 size_t i;
 int ret;
 bool functionReturn = false;
-unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
-bool current = vshCommandOptBool(cmd, "current");
-bool config = vshCommandOptBool(cmd, "config");
-bool live = vshCommandOptBool(cmd, "live");
-bool persistent = vshCommandOptBool(cmd, "persistent");
-
-VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
-
-VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
-VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
-
-if (config || persistent)
-flags |= VIR_DOMAIN_AFFECT_CONFIG;
-if (live)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-return false;
 
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 goto cleanup;
@@ -10859,15 +10823,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "mac", ) < 0)
 goto cleanup;
 
-if (persistent &&
-virDomainIsActive(dom) == 1)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-if (flags & VIR_DOMAIN_AFFECT_CONFIG)
-doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
-else
-doc = virDomainGetXMLDesc(dom, 0);
-
 if (!doc)
 goto cleanup;
 
@@ -10935,20 +10890,72 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 else
 ret = virDomainDetachDevice(dom, detach_xml);
 
-if (ret != 0) {
-vshError(ctl, "%s", _("Failed to detach interface"));
-} else {
-vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+if (ret == 0) 
 functionReturn = true;
-}
 
  cleanup:
-VIR_FREE(doc);
 VIR_FREE(detach_xml);
-virDomainFree(dom);
+xmlFreeDoc(xml);
 xmlXPathFreeObject(obj);
 xmlXPathFreeContext(ctxt);
-xmlFreeDoc(xml);
+return functionReturn;
+}
+
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+char *doc_live = NULL, *doc_config = NULL;
+bool current = vshCommandOptBool(cmd, "current");
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+bool persistent = vshCommandOptBool(cmd, "persistent");
+bool functionReturn = false;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config || persistent)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (persistent &&
+virDomainIsActive(dom) == 1)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
+if (!(functionReturn = vshDomainDetachInterface(doc_config, flags, 
dom, ctl, cmd)))
+goto cleanup;
+} 
+
+if (live || (!live && !config))
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+doc_live = virDomainGetXMLDesc(dom, 0);
+  
+if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+flags &= (~VIR_DOMAIN_AFFECT_CONFIG); 
+
+functionReturn = vshDomainDetachInterface(doc_live, flags, dom, ctl, 
cmd);
+}
+
+ cleanup:
+if (functionReturn == false) {
+vshError(ctl, "%s", _("Failed to detach interface"));
+} else {
+vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+functionReturn = true;
+}
+VIR_FREE(doc_live); 
+VIR_FREE(doc_config);
+virDomainFree(dom);
 return functionReturn;
 }
 

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

Re: [libvirt] [PATCH] Use correct pci addresses during interface-detach[v2]

[Nitesh Konkar]

Hello Michal,

Thanks for the feedback. I have sent a follow up v3 version of my patch.

Warm Regards,
Nitesh Konkar.


On Mon, Feb 22, 2016 at 6:02 PM, Michal Privoznik 
wrote:

> On 19.02.2016 12:53, Nitesh Konkar wrote:
> > The virsh attach virsh detach interface command fails  when both live
> and config
> > are set and when the  interface gets attached to different pci slots
> > on live and config xml respectively.
> >
> > When we attach an interface with both --live and --config,
> > the first time they get the same PCI slots, but the second time
> > onwards it differs and hence the virsh detach-interface --live
> > --config command fails. This patch makes sure that when both
> > --live --config are set , qemuDomainDetachDeviceFlags is called
> > twice, once with config xml and once with live xml.
> >
> > Steps to see the issue:
> > virsh attach-interface --domain DomainName --type network --source
> default --mac 52:54:00:4b:76:5f --live --config
> > virsh  detach-interface --domain DomainName --type network --mac
> 52:54:00:4b:76:5f --live --config
> > virsh attach-interface --domain DomainName --type network --source
> default --mac 52:54:00:4b:76:5f --live --config
> > virsh  detach-interface --domain DomainName --type network --mac
> 52:54:00:4b:76:5f --live --config
> >
>
> Okay, I can see the problem but I find the solution a bit hackish.
>
> > Signed-off-by:nitko...@linux.vnet.ibm.com
> > ---
> >  tools/virsh-domain.c | 14 --
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 62acecb..43c8436 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -10815,7 +10815,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd
> *cmd)
> >  char buf[64];
> >  int diff_mac;
> >  size_t i;
> > -int ret;
> > +int ret, flag_live_config_both = 0;
>
> This new flag makes me confused. I know what you're trying to achieve
> with it, but the name could be better. How about configDetached?
> Moreover, it's a boolean not an int.
>
> >  bool functionReturn = false;
> >  unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> >  bool current = vshCommandOptBool(cmd, "current");
> > @@ -10830,7 +10830,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd
> *cmd)
> >
> >  if (config || persistent)
> >  flags |= VIR_DOMAIN_AFFECT_CONFIG;
> > -if (live)
> > +if (!(config || persistent) && live) // Only Live
> >  flags |= VIR_DOMAIN_AFFECT_LIVE;
>
> I don't think I follow. But maybe I'm misguided by --persistent.
> Historically it just an alias for --config. But not in this case. I
> wonder why.
>
> >
> >  if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> > @@ -10851,6 +10851,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd
> *cmd)
> >  else
> >  doc = virDomainGetXMLDesc(dom, 0);
> >
> > +forlivexml:
> > +
>
> I'd rather avoid introducing this kind of label. How about putting the
> important code (interface lookup in domain XML) into a separate function
> and call it twice if needed? That way you can also avoid using
> @flag_live_config_both.
>
> >  if (!doc)
> >  goto cleanup;
> >
> > @@ -10921,6 +10923,14 @@ cmdDetachInterface(vshControl *ctl, const
> vshCmd *cmd)
> >  if (ret != 0) {
> >  vshError(ctl, "%s", _("Failed to detach interface"));
> >  } else {
> > +if ((config || persistent) && live && flag_live_config_both ==
> 0) {//executed only when live config both in cmd.
> > + doc = virDomainGetXMLDesc(dom, 0);
> > + flag_live_config_both=1; //Need to execute this code only
> once
> > + flags |= VIR_DOMAIN_AFFECT_LIVE; //set live flag
> > + flags &= (~VIR_DOMAIN_AFFECT_CONFIG ); // need to unset
> config flag as config xml detach is already done.
> > + mac=NULL;
> > + goto forlivexml;
> > + }
> >  vshPrint(ctl, "%s", _("Interface detached successfully\n"));
> >  functionReturn = true;
> >  }
> >
>
> Then, this patch does not comply with our formatting rules. Run 'make
> syntax-check' to see all the problems.
>
> Looking forward to v3.
>
> Michal
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3] Use correct pci addresses during interface-detach

[Nitesh Konkar]

From: Nitesh_Konkar 

---
 tools/virsh-domain.c | 103 +--
 1 file changed, 64 insertions(+), 39 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 979f115..a15ef40 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
 
-static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+static bool 
+vshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, 
vshControl *ctl, const vshCmd *cmd)
 {
-virDomainPtr dom = NULL;
 xmlDocPtr xml = NULL;
 xmlXPathObjectPtr obj = NULL;
 xmlXPathContextPtr ctxt = NULL;
 xmlNodePtr cur = NULL, matchNode = NULL;
-char *detach_xml = NULL;
 const char *mac = NULL, *type = NULL;
-char *doc = NULL;
+char *detach_xml = NULL;
+bool current = vshCommandOptBool(cmd, "current");
 char buf[64];
 int diff_mac;
 size_t i;
 int ret;
 bool functionReturn = false;
-unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
-bool current = vshCommandOptBool(cmd, "current");
-bool config = vshCommandOptBool(cmd, "config");
-bool live = vshCommandOptBool(cmd, "live");
-bool persistent = vshCommandOptBool(cmd, "persistent");
-
-VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
-
-VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
-VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
-
-if (config || persistent)
-flags |= VIR_DOMAIN_AFFECT_CONFIG;
-if (live)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-return false;
 
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 goto cleanup;
@@ -10859,15 +10823,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "mac", ) < 0)
 goto cleanup;
 
-if (persistent &&
-virDomainIsActive(dom) == 1)
-flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-if (flags & VIR_DOMAIN_AFFECT_CONFIG)
-doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
-else
-doc = virDomainGetXMLDesc(dom, 0);
-
 if (!doc)
 goto cleanup;
 
@@ -10935,20 +10890,72 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 else
 ret = virDomainDetachDevice(dom, detach_xml);
 
-if (ret != 0) {
-vshError(ctl, "%s", _("Failed to detach interface"));
-} else {
-vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+if (ret == 0) 
 functionReturn = true;
-}
 
  cleanup:
-VIR_FREE(doc);
 VIR_FREE(detach_xml);
-virDomainFree(dom);
+xmlFreeDoc(xml);
 xmlXPathFreeObject(obj);
 xmlXPathFreeContext(ctxt);
-xmlFreeDoc(xml);
+return functionReturn;
+}
+
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+char *doc_live = NULL, *doc_config = NULL;
+bool current = vshCommandOptBool(cmd, "current");
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+bool persistent = vshCommandOptBool(cmd, "persistent");
+bool functionReturn = false;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config || persistent)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (persistent &&
+virDomainIsActive(dom) == 1)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
+if (!(functionReturn = vshDomainDetachInterface(doc_config, flags, 
dom, ctl, cmd)))
+goto cleanup;
+} 
+
+if (live || (!live && !config))
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+doc_live = virDomainGetXMLDesc(dom, 0);
+  
+if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+flags &= (~VIR_DOMAIN_AFFECT_CONFIG); 
+
+functionReturn = vshDomainDetachInterface(doc_live, flags, dom, ctl, 
cmd);
+}
+
+ cleanup:
+if (functionReturn == false) {
+vshError(ctl, "%s", _("Failed to detach interface"));
+} else {
+vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+functionReturn = true;
+}
+VIR_FREE(doc_live); 
+VIR_FREE(doc_config);
+virDomainFree(dom);
 return functionReturn;
 }
 

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