Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

[Chen, Hanxiao]

> -Original Message-
> From: Richard Weinberger [mailto:rich...@nod.at]
> Sent: Friday, March 20, 2015 1:41 AM
> To: Daniel P. Berrange
> Cc: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable 
> userns
> but disable netns
> 
> Am 19.03.2015 um 18:28 schrieb Daniel P. Berrange:
> > On Thu, Mar 19, 2015 at 06:04:57PM +0100, Richard Weinberger wrote:
> >> Am 19.03.2015 um 17:58 schrieb Daniel P. Berrange:
> >>> On Thu, Mar 19, 2015 at 05:54:32PM +0100, Richard Weinberger wrote:
>  Am 11.03.2015 um 10:36 schrieb Richard Weinberger:
> > Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao:
>  @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool
> userns_enabled)
>   bool bindOverReadonly;
>   virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
> 
>  +/* When enable userns but disable netns, kernel will
>  + * forbid us doing a new fresh mount for sysfs.
>  + * So we had to do a bind mount for sysfs instead.
>  + */
>  +if (userns_enabled && netns_disabled &&
>  +STREQ(mnt->src, "sysfs")) {
>  +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
>  +goto cleanup;
>  +}
> >>>
> >>> This is clearly broken and looks very untested to me.
> >>>
> >> It's broken now.
> >> But when I submitted this patch last year, it's not.
> >
> > Are you sure?
> > Just built libvirt v1.2.6-222-ga86b621, head is
> > commit a86b6215a74b1feb2667204e214fbfd2f7decc5c
> > Author: Chen Hanxiao 
> > Date:   Mon Jul 14 18:01:51 2014 +0800
> >
> > LXC: create a bind mount for sysfs when enable userns but disable 
> > netns
> >
> > /sys is still an empty directory but as at this time (most likely due 
> > to another
> bug)
> > libvirt was able to create /sys/fs/cgroup and mounted groups there.
> > But no sysfs at all is at /sys.
> >
> > I mean, how is this supposed to work? You bind mount /sys over /sys...
> 
>  Any further comments on that?
> >>>
> >>> It just looks impossible for it to work in this way
> >>
> >> That's also my impression.
> >>
> >> Therefore containers without their own network namespace currently don't 
> >> work
> >> and have never worked as expected.
> >
> > No, it is only a problem if userns is used. If userns is not used then
> > they do work
> 
> Agreed.
> 
That's what I tried to do.
Sorry for my mistake.

> >> Shall we revert commit a86b6215a74b and try to bind mount
> >> before the pivot_root()?
> >
> > Not sure if that works with userns is active either.
> 
> Fact is that commit a86b6215a74 is broken.
> We could also refuse to create container with userns enabled but netns 
> disabled...
> 

I think we should refuse it too, rather than do something to work around.
Dan, what's your opinion?

Regards,
- Chen

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

Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug

[Zhu Guihua]

On 03/20/2015 06:39 AM, John Ferlan wrote:


On 03/18/2015 01:51 AM, Zhu Guihua wrote:

On 03/17/2015 10:19 PM, Peter Krempa wrote:

Add a few helpers that allow to operate with memory device definitions
on the domain config and use them to implement memory device coldplug in
the qemu driver.
---

Notes:
  Version 2:
  - no changes

   src/conf/domain_conf.c   | 100
+++
   src/conf/domain_conf.h   |  10 +
   src/libvirt_private.syms |   4 ++
   src/qemu/qemu_driver.c   |  15 ++-
   4 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c2234f..1a02e46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def,
   }


+static int
+virDomainMemoryFindByDefInternal(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem,
+ bool allowAddressFallback)
+{
+size_t i;
+
+for (i = 0; i < def->nmems; i++) {
+virDomainMemoryDefPtr tmp = def->mems[i];
+
+/* address, if present */
+if (allowAddressFallback) {
+if (tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+continue;
+} else {
+if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+!virDomainDeviceInfoAddressIsEqual(&tmp->info,
&mem->info))
+continue;
+}
+
+/* alias, if present */
+if (mem->info.alias &&
+STRNEQ_NULLABLE(tmp->info.alias, mem->info.alias))
+continue;
+
+/* target info -> always present */
+if (tmp->model != mem->model ||
+tmp->targetNode != mem->targetNode ||
+tmp->size != mem->size)

I have tested your series with our qemu memory hot remove patch series,
here would be a possible error.

When hotplug a memory device, its size has been aligned. So the compare for
size here would fail possiblely.


hmm.. Not sure that's necessary - although Peter can make the final
determination... Commit id '57b215a' doesn't modify each def->mems[i]
entry in qemuDomainAlignMemorySizes, rather it gets a value from
virDomainDefSetMemoryInitial and then does the rounding.

If the stored def->mems[i]->size value is/was modified, then I'd agree,
but it doesn't appear to be that way.

If there is a rounding of the value - then please just point it out


Yes, the stored def->mems[i]->size value was modified.
If you assign the size 524287 KiB, the stored value will be 524288.

Thanks,
Zhu



John


Thanks,
Zhu


+continue;
+
+/* source stuff -> match with device */
+if (tmp->pagesize != mem->pagesize)
+continue;
+
+if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes))
+continue;
+
+break;
+}
+
+if (i == def->nmems)
+return -1;
+
+return i;
+}
+
+
+int
+virDomainMemoryFindByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+return virDomainMemoryFindByDefInternal(def, mem, false);
+}
+

[...]

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

.



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

Re: [libvirt] [PATCH] qemu_command.c: allow hostdev to be attached to PCIe bus

[Laine Stump]

On 03/19/2015 01:42 PM, Steven Walter wrote:
> Presently attaching a host device to the pcie-root controller fails, even if
> the host device is actually a PCIe device.  With this change, the attachment
> succeeds, QEMU is happy, and the guest correctly sees the host device as a
> child of the PCIe root.
> ---
>  src/qemu/qemu_command.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4d1590c..1de89e3 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1377,6 +1377,13 @@ qemuCollectPCIAddress(virDomainDefPtr def 
> ATTRIBUTE_UNUSED,
>   */
>  flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
>  break;
> +
> +case VIR_DOMAIN_DEVICE_HOSTDEV:
> +/* hostdev aren't hot-plugged, and can be put in either a
> + * PCI or PCIe slot
> + */
> +flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
> +break;
>  }
>  
>  /* Ignore implicit controllers on slot 0:0:1.0:

When adding support for the PCIE controller, I explicitly blocked this
because there wasn't concrete information on what was and wasn't
supported by qemu (as opposed to just currently working by fluke of
fate). Before we push anything like this, we should have an updated
discussion with qemu people to verify what can be connected to who, and
whether or not hot-plug is supported, and maybe try to do something a
bit more comprehensive.

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

Re: [libvirt] [PATCH 2/2] qemu: Migrate memory on numatune change

[John Ferlan]

On 03/11/2015 08:39 AM, Martin Kletzander wrote:
> We've never set the cpuset.memory_migrate value to anything, keeping it
> on default.  However, we allow changing cpuset.mems on live domain.
> That setting, however, don't have any consequence on a domain unless
> it's going to allocate new memory.
> 
> I managed to make 'virsh numatune' move all the memory to any node I
> wanted even without disabling libnuma's numa_set_membind(), so this
> should be safe to use with it as well.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1198497
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/qemu/qemu_cgroup.c | 36 +++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 2b5a182..976a8c4 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -1,7 +1,7 @@
>  /*
>   * qemu_cgroup.c: QEMU cgroup management
>   *
> - * Copyright (C) 2006-2014 Red Hat, Inc.
> + * Copyright (C) 2006-2015 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -652,6 +652,9 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
>  if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
>  return 0;
> 
> +if (virCgroupSetCpusetMemoryMigrate(priv->cgroup, true) < 0)
> +return -1;
> +

Is this OK to set even if NUMA isn't enabled?  I didn't follow the
callers all the way back and this isn't something I know a lot about, so
I'm curious. I do see from the man page it's available since Linux
2.6.16, so one would hope/think that we're OK in assuming we can access
it...

Is it possible that someone wouldn't want to migrate the memory?  That
is should this be a configurable attribute?



>  if (vm->def->cpumask ||
>  (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)) {
> 
> @@ -792,9 +795,12 @@ static void
>  qemuRestoreCgroupState(virDomainObjPtr vm)
>  {
>  char *mem_mask = NULL;
> +char *nodeset = NULL;
>  int empty = -1;
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> +size_t i = 0;
>  virBitmapPtr all_nodes;
> +virCgroupPtr cgroup_temp = NULL;
> 
>  if (!(all_nodes = virNumaGetHostNodeset()))
>  goto error;
> @@ -809,9 +815,37 @@ qemuRestoreCgroupState(virDomainObjPtr vm)

So this is the path for currently running guests to be adjusted on
livirtd restart after install, right?

>  if (virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0)
>  goto error;
> 
> +for (i = 0; i < priv->nvcpupids; i++) {
> +if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_temp) < 0 ||
> +virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 ||
> +virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 ||
> +virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0)
> +goto cleanup;
> +
> +virCgroupFree(&cgroup_temp);
> +}
> +
> +for (i = 0; i < priv->niothreadpids; i++) {
> +if (virCgroupNewIOThread(priv->cgroup, i, false, &cgroup_temp) < 0 ||

cgroup iothread id's are 1..n, so I believe this should be 'i + 1'

simple enough to test by adding 2 to your domain
definition and making sure this works...

Beyond that - seems reasonable to me

John
> +virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 ||
> +virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 ||
> +virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0)
> +goto cleanup;
> +
> +virCgroupFree(&cgroup_temp);
> +}
> +
> +if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0 ||
> +virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 ||
> +virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 ||
> +virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0)
> +goto cleanup;
> +
>   cleanup:
>  VIR_FREE(mem_mask);
> +VIR_FREE(nodeset);
>  virBitmapFree(all_nodes);
> +virCgroupFree(&cgroup_temp);
>  return;
> 
>   error:
> 

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

Re: [libvirt] [PATCH 1/2] cgroup: Add accessors for cpuset.memory_migrate

[Eric Blake]

On 03/19/2015 05:11 PM, John Ferlan wrote:

>> +int
>> +virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate)
>> +{
>> +return virCgroupSetValueStr(group,
>> +VIR_CGROUP_CONTROLLER_CPUSET,
>> +"cpuset.memory_migrate",
>> +migrate ? "1" : "0");
>> +}
> 
> are my eyes deceiving me?...  boolean here (1 or 0)

No, the kernel really works this way - it takes the human-readable
string "1" or "0" rather than the C byte '\1' or '\0', so this is the
lowest level in the stack where we convert our convenient bool for
everyone above us into the string literal the kernel cgroup interface
expects.

>> +int
>> +virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate)
>> +{
>> +unsigned long long value = 0;
>> +int ret = virCgroupGetValueU64(group,
> 
> But we're getting a U64 here? for what's documented as "contains a flag
> (0 or 1)
> 
>> +   VIR_CGROUP_CONTROLLER_CPUSET,
>> +   "cpuset.memory_migrate",
>> +   &value);
>> +*migrate = !!value;

And here, this is one easy way to turn the kernel's string back into a
bool (it might also be possible to read as a string, and then do
*str=='1', rather than going through an intermediate integer conversion,
but I'm not sure it is worth the micro-optimization).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 1/2] cgroup: Add accessors for cpuset.memory_migrate

[John Ferlan]

On 03/11/2015 08:39 AM, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander 
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/vircgroup.c | 42 +-
>  src/util/vircgroup.h |  5 -
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e0d5459..f3d8517 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1130,6 +1130,7 @@ virCgroupGetCpuacctUsage;
>  virCgroupGetCpuCfsPeriod;
>  virCgroupGetCpuCfsQuota;
>  virCgroupGetCpusetCpus;
> +virCgroupGetCpusetMemoryMigrate;
>  virCgroupGetCpusetMems;
>  virCgroupGetCpuShares;
>  virCgroupGetDevicePermsString;
> @@ -1170,6 +1171,7 @@ virCgroupSetBlkioWeight;
>  virCgroupSetCpuCfsPeriod;
>  virCgroupSetCpuCfsQuota;
>  virCgroupSetCpusetCpus;
> +virCgroupSetCpusetMemoryMigrate;
>  virCgroupSetCpusetMems;
>  virCgroupSetCpuShares;
>  virCgroupSetFreezerState;
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 6957e81..5d3de10 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -1,7 +1,7 @@
>  /*
>   * vircgroup.c: methods for managing control cgroups
>   *
> - * Copyright (C) 2010-2014 Red Hat, Inc.
> + * Copyright (C) 2010-2015 Red Hat, Inc.
>   * Copyright IBM Corp. 2008
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -872,6 +872,7 @@ virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr 
> group)
>  const char *inherit_values[] = {
>  "cpuset.cpus",
>  "cpuset.mems",
> +"cpuset.memory_migrate",
>  };
> 
>  VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path);
> @@ -2671,6 +2672,45 @@ virCgroupGetCpusetMems(virCgroupPtr group, char **mems)
> 
> 
>  /**
> + * virCgroupSetCpusetMemoryMigrate:
> + *
> + * @group: The cgroup to set cpuset.memory_migrate for
> + * @migrate: Whether to migrate the memory on change or not
> + *
> + * Returns: 0 on success
> + */
> +int
> +virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate)
> +{
> +return virCgroupSetValueStr(group,
> +VIR_CGROUP_CONTROLLER_CPUSET,
> +"cpuset.memory_migrate",
> +migrate ? "1" : "0");
> +}

are my eyes deceiving me?...  boolean here (1 or 0)

> +
> +
> +/**
> + * virCgroupGetCpusetMemoryMigrate:
> + *
> + * @group: The cgroup to get cpuset.memory_migrate for
> + * @migrate: Migration setting
> + *
> + * Returns: 0 on success
> + */
> +int
> +virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate)
> +{
> +unsigned long long value = 0;
> +int ret = virCgroupGetValueU64(group,

But we're getting a U64 here? for what's documented as "contains a flag
(0 or 1)

> +   VIR_CGROUP_CONTROLLER_CPUSET,
> +   "cpuset.memory_migrate",
> +   &value);
> +*migrate = !!value;
> +return ret;
> +}

Also there's no callers to this? I see that vircgrouptest.c exists and
there are a few API's called there - of course none for the
virCgroupGetCpuset*...

I think this is "ACKable" with perhaps usage of the right API to get at
the value.

John
> +
> +
> +/**
>   * virCgroupSetCpusetCpus:
>   *
>   * @group: The cgroup to set cpuset.cpus for
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index 9f984e7..bfb3a9b 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -1,7 +1,7 @@
>  /*
>   * vircgroup.h: methods for managing control cgroups
>   *
> - * Copyright (C) 2011-2014 Red Hat, Inc.
> + * Copyright (C) 2011-2015 Red Hat, Inc.
>   * Copyright IBM Corp. 2008
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -251,6 +251,9 @@ int virCgroupGetFreezerState(virCgroupPtr group, char 
> **state);
>  int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems);
>  int virCgroupGetCpusetMems(virCgroupPtr group, char **mems);
> 
> +int virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate);
> +int virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate);
> +
>  int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus);
>  int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus);
> 

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

Re: [libvirt] [PATCHv3 11/12] qemu: Implement memory device hotplug

[John Ferlan]

On 03/17/2015 10:20 AM, Peter Krempa wrote:
> Add code to hot-add memory devices to running qemu instances.
> ---
> 
> Notes:
> Version 3:
> - added comment to clarify that @mem is always consumed by 
> qemuDomainAttachMemory
> Version 2:
> - no change
> 
> Version 2:
> - no change
> 
>  src/qemu/qemu_command.c |  4 +--
>  src/qemu/qemu_command.h | 15 
>  src/qemu/qemu_driver.c  |  5 ++-
>  src/qemu/qemu_hotplug.c | 95 
> +
>  src/qemu/qemu_hotplug.h |  3 ++
>  5 files changed, 119 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2d85567..1f72437 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4599,7 +4599,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>   * other configuration was used (to detect legacy configurations). Returns
>   * -1 in case of an error.
>   */
> -static int
> +int
>  qemuBuildMemoryBackendStr(unsigned long long size,
>unsigned long long pagesize,
>int guestNode,
> @@ -4872,7 +4872,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
>  }
> 
> 
> -static char *
> +char *
>  qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
>   virQEMUCapsPtr qemuCaps)
>  {
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index ee81f92..a29db41 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -162,6 +162,21 @@ char *qemuBuildSoundDevStr(virDomainDefPtr domainDef,
> virDomainSoundDefPtr sound,
> virQEMUCapsPtr qemuCaps);
> 
> +int qemuBuildMemoryBackendStr(unsigned long long size,
> +  unsigned long long pagesize,
> +  int guestNode,
> +  virBitmapPtr userNodeset,
> +  virBitmapPtr autoNodeset,
> +  virDomainDefPtr def,
> +  virQEMUCapsPtr qemuCaps,
> +  virQEMUDriverConfigPtr cfg,
> +  const char **backendType,
> +  virJSONValuePtr *backendProps,
> +  bool force);
> +
> +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
> +   virQEMUCapsPtr qemuCaps);
> +
>  /* Legacy, pre device support */
>  char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev,
> virQEMUCapsPtr qemuCaps);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e948cca..cbdf279 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7649,8 +7649,11 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>  dev->data.rng = NULL;
>  break;
> 
> -/*TODO: implement later */
>  case VIR_DOMAIN_DEVICE_MEMORY:
> +ret = qemuDomainAttachMemory(driver, vm,
> + dev->data.memory);
> +dev->data.memory = NULL;
> +break;

FWIW: Although there is a note in the AttachMemory code about
consumption it may be worth noting here too just so no one in the future
"sees" it missing and adds it thinking it's necessary

> 
>  case VIR_DOMAIN_DEVICE_NONE:
>  case VIR_DOMAIN_DEVICE_FS:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 5c5ad0e..88c5e3c 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1672,6 +1672,101 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
>  }
> 
> 
> +/**
> + * qemuDomainAttachMemory:
> + * @driver: qemu driver data
> + * @vm: VM object
> + * @mem: Definition of the memory device to be attached. @mem is always 
> consumed
> + *
> + * Attaches memory device described by @mem to domain @vm.
> + *
> + * Returns 0 on success -1 on error.
> + */
> +int
> +qemuDomainAttachMemory(virQEMUDriverPtr driver,
> +   virDomainObjPtr vm,
> +   virDomainMemoryDefPtr mem)
> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +char *devstr = NULL;
> +char *objalias = NULL;
> +const char *backendType;
> +virJSONValuePtr props = NULL;
> +int id;
> +int ret = -1;
> +
> +if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems) < 0)
> +goto cleanup;
> +
> +if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0)
> +goto cleanup;
> +
> +if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv->qemuCaps)))
> +goto cleanup;
> +
> +qemuDomainMemoryDeviceAlignSize(mem);
> +
> +if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize,
> +  mem->targetNode, mem->sourceNodes, NULL,
> +  vm->def, priv->qemuCaps, cfg,
> +   

Re: [libvirt] [PATCHv3 00/12] Add support for memory hotplug

[John Ferlan]

On 03/17/2015 10:19 AM, Peter Krempa wrote:
> This version includes review feedback changes from John and also fixes the
>  element documentation and code that calculates it to support 
> possible 
> non-NUMA configs with memory hotplug if any hypervisor would support that in 
> the
> future.
> 
> Peter Krempa (12):
>   qemu: monitor: Don't leak @props with non-JSON in qemuMonitorAddObject
>   libxl: Refactor logic in domain post parse callback
>   conf: Add support for parsing and formatting max memory and slot count
>   qemu: Implement setup of memory hotplug parameters
>   conf: Add device address type for dimm devices
>   conf: Add interface to parse and format memory device information
>   qemu: memdev: Add infrastructure to load memory device information
>   qemu: migration: Forbid migration with memory modules lacking info
>   qemu: add support for memory devices
>   qemu: conf: Add support for memory device cold(un)plug
>   qemu: Implement memory device hotplug
>   qemu: Implement memory device hotunplug
> 
>  docs/formatdomain.html.in  | 112 +++-
>  docs/schemas/domaincommon.rng  |  76 +++
>  src/bhyve/bhyve_domain.c   |   9 +-
>  src/conf/domain_conf.c | 576 
> -
>  src/conf/domain_conf.h |  59 +++
>  src/libvirt_private.syms   |   7 +
>  src/libxl/libxl_domain.c   |  15 +-
>  src/lxc/lxc_domain.c   |   8 +
>  src/openvz/openvz_driver.c |  14 +-
>  src/parallels/parallels_driver.c   |   6 +-
>  src/phyp/phyp_driver.c |   6 +-
>  src/qemu/qemu_command.c| 166 +-
>  src/qemu/qemu_command.h|  15 +
>  src/qemu/qemu_domain.c |  80 +++
>  src/qemu/qemu_domain.h |   5 +
>  src/qemu/qemu_driver.c |  29 ++
>  src/qemu/qemu_hotplug.c| 187 +++
>  src/qemu/qemu_hotplug.h|   6 +
>  src/qemu/qemu_migration.c  |  14 +
>  src/qemu/qemu_monitor.c|  48 +-
>  src/qemu/qemu_monitor.h|  14 +
>  src/qemu/qemu_monitor_json.c   | 122 +
>  src/qemu/qemu_monitor_json.h   |   5 +
>  src/qemu/qemu_process.c|   4 +
>  src/uml/uml_driver.c   |   9 +-
>  src/vbox/vbox_common.c |   6 +-
>  src/vmware/vmware_driver.c |   6 +-
>  src/vmx/vmx.c  |   6 +-
>  src/xen/xen_driver.c   |   7 +
>  src/xenapi/xenapi_driver.c |   9 +-
>  tests/domainschemadata/maxMemory.xml   |  19 +
>  .../qemuxml2argv-memory-hotplug-dimm.args  |  11 +
>  .../qemuxml2argv-memory-hotplug-dimm.xml   |  50 ++
>  .../qemuxml2argv-memory-hotplug-nonuma.xml |  22 +
>  .../qemuxml2argv-memory-hotplug.args   |   6 +
>  .../qemuxml2argv-memory-hotplug.xml|  34 ++
>  tests/qemuxml2argvtest.c   |   6 +
>  tests/qemuxml2xmltest.c|   4 +
>  38 files changed, 1747 insertions(+), 31 deletions(-)
>  create mode 100644 tests/domainschemadata/maxMemory.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nonuma.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug.xml
> 

ACK series with adjustments noted in patches 9 & 11 and of course the
assurance about patch 10.

John

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

Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug

[John Ferlan]

On 03/18/2015 01:51 AM, Zhu Guihua wrote:
> 
> On 03/17/2015 10:19 PM, Peter Krempa wrote:
>> Add a few helpers that allow to operate with memory device definitions
>> on the domain config and use them to implement memory device coldplug in
>> the qemu driver.
>> ---
>>
>> Notes:
>>  Version 2:
>>  - no changes
>>
>>   src/conf/domain_conf.c   | 100
>> +++
>>   src/conf/domain_conf.h   |  10 +
>>   src/libvirt_private.syms |   4 ++
>>   src/qemu/qemu_driver.c   |  15 ++-
>>   4 files changed, 127 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 8c2234f..1a02e46 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def,
>>   }
>>
>>
>> +static int
>> +virDomainMemoryFindByDefInternal(virDomainDefPtr def,
>> + virDomainMemoryDefPtr mem,
>> + bool allowAddressFallback)
>> +{
>> +size_t i;
>> +
>> +for (i = 0; i < def->nmems; i++) {
>> +virDomainMemoryDefPtr tmp = def->mems[i];
>> +
>> +/* address, if present */
>> +if (allowAddressFallback) {
>> +if (tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
>> +continue;
>> +} else {
>> +if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
>> +!virDomainDeviceInfoAddressIsEqual(&tmp->info,
>> &mem->info))
>> +continue;
>> +}
>> +
>> +/* alias, if present */
>> +if (mem->info.alias &&
>> +STRNEQ_NULLABLE(tmp->info.alias, mem->info.alias))
>> +continue;
>> +
>> +/* target info -> always present */
>> +if (tmp->model != mem->model ||
>> +tmp->targetNode != mem->targetNode ||
>> +tmp->size != mem->size)
> 
> I have tested your series with our qemu memory hot remove patch series,
> here would be a possible error.
> 
> When hotplug a memory device, its size has been aligned. So the compare for
> size here would fail possiblely.
> 

hmm.. Not sure that's necessary - although Peter can make the final
determination... Commit id '57b215a' doesn't modify each def->mems[i]
entry in qemuDomainAlignMemorySizes, rather it gets a value from
virDomainDefSetMemoryInitial and then does the rounding.

If the stored def->mems[i]->size value is/was modified, then I'd agree,
but it doesn't appear to be that way.

If there is a rounding of the value - then please just point it out

John

> Thanks,
> Zhu
> 
>> +continue;
>> +
>> +/* source stuff -> match with device */
>> +if (tmp->pagesize != mem->pagesize)
>> +continue;
>> +
>> +if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes))
>> +continue;
>> +
>> +break;
>> +}
>> +
>> +if (i == def->nmems)
>> +return -1;
>> +
>> +return i;
>> +}
>> +
>> +
>> +int
>> +virDomainMemoryFindByDef(virDomainDefPtr def,
>> + virDomainMemoryDefPtr mem)
>> +{
>> +return virDomainMemoryFindByDefInternal(def, mem, false);
>> +}
>> +
> [...]
> 
> -- 
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH] libxl: use xenlight pkgconfig file if present

[Jim Fehlig]

Michal Privoznik wrote:
> On 18.03.2015 23:25, Jim Fehlig wrote:
>   
>> xen.git commit babeca32 added a pkgconfig file for libxenlight,
>> allowing libxl apps to determine the location of Xen binaries
>> such as firmware blobs, device emulator, etc.
>>
>> This patch adds support for xenlight.pc in the libxl driver, falling
>> back to the previous configure logic if not found.  It introduces
>> LIBXL_FIRMWARE_DIR and LIBXL_EXECBIN_DIR to define the firmware and
>> libexec_bin locations.  If xenlight.pc does not exist, the defines
>> are set to the current hardcoded paths.  The capabilities'
>>  and  elements are updated to use the paths.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>  configure.ac   | 47 +--
>>  src/libxl/libxl_conf.c |  7 ++-
>>  src/libxl/libxl_conf.h |  8 
>>  3 files changed, 43 insertions(+), 19 deletions(-)
>> 
>
>   
> ACK

Thanks; pushed.

Regards,
Jim

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

Re: [libvirt] [PATCHv3 09/12] qemu: add support for memory devices

[John Ferlan]

On 03/17/2015 10:19 AM, Peter Krempa wrote:
> Add support to start qemu instance with 'pc-dimm' device. Thanks to the
> refactors we are able to reuse the existing function to determine the
> parameters.
> ---
> 
> Notes:
> Version 2:
> - dropped the ACPI naming
> 
>  src/qemu/qemu_command.c| 130 
> -
>  src/qemu/qemu_domain.c |  26 -
>  src/qemu/qemu_domain.h |   1 +
>  .../qemuxml2argv-memory-hotplug-dimm.args  |  11 ++
>  tests/qemuxml2argvtest.c   |   2 +
>  tests/qemuxml2xmltest.c|   1 +
>  6 files changed, 167 insertions(+), 4 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ef8feeb..2d85567 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1222,6 +1222,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, 
> virQEMUCapsPtr qemuCaps)
>  if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0)
>  return -1;
>  }
> +for (i = 0; i < def->nmems; i++) {
> +if (virAsprintf(&def->mems[i]->info.alias, "dimm%zu", i) < 0)
> +return -1;
> +}
> 
>  return 0;
>  }
> @@ -4612,8 +4616,7 @@ qemuBuildMemoryBackendStr(unsigned long long size,
>  virDomainHugePagePtr hugepage = NULL;
>  virDomainNumatuneMemMode mode;
>  const long system_page_size = virGetSystemPageSizeKB();
> -virNumaMemAccess memAccess = 
> virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
> -
> +virNumaMemAccess memAccess = VIR_NUMA_MEM_ACCESS_DEFAULT;
>  size_t i;
>  char *mem_path = NULL;
>  virBitmapPtr nodemask = NULL;
> @@ -4626,6 +4629,16 @@ qemuBuildMemoryBackendStr(unsigned long long size,
>  if (!(props = virJSONValueNewObject()))
>  return -1;
> 
> +/* memory devices could provide a invalid guest node */
> +if (guestNode >= virDomainNumaGetNodeCount(def->numa)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("can't add memory backend for guest node '%d' as "
> + "the guest has only '%zu' NUMA nodes configured"),
> +   guestNode, virDomainNumaGetNodeCount(def->numa));

Coverity points out that 'props' is being leaked here.  So probably
should go to cleanup or move this entire hunk above the props = line.

In my test I just moved these lines above the props fetch and Coverity
was happy

John

> +return -1;
> +}
> +
> +memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
>  mode = virDomainNumatuneGetMode(def->numa, guestNode);
> 
>  if (pagesize == 0 || pagesize != system_page_size) {
> @@ -4823,6 +4836,95 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
>  }
> 
> 

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

[libvirt] [RFC PATCH 0/2] Vbox: Add support for virDomainSendKey

[Dawid Zamirski]

Hello,

Those small patches implement virDomainSendKey support in VBOX driver. However,
the VBOX SDK does not support "holdtime" so I used usleep to wait for that time
before sending "key-up" scancodes. This makes it behave similarly to QEMU
driver, however I'm not sure if that way of handling this would be preferred.
Another option, would be to ignore holdtime argument and make virDomainSendKey 
work the same as via VBoxManage cli tool where one has to send
"key-down" scancodes followed by "key-up" scancodes. For this RFC paches, I've
choosen to make it work as close to the public API documentation.


Dawid Zamirski (2):
  vbox: Register IKeyboard with the unified API.
  vbox: Implement virDomainSendKey

 src/vbox/vbox_common.c| 107 ++
 src/vbox/vbox_common.h|   1 +
 src/vbox/vbox_tmpl.c  |  27 +++
 src/vbox/vbox_uniformed_api.h |   8 
 4 files changed, 143 insertions(+)

-- 
2.3.3

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

[libvirt] [RFC PATCH 2/2] vbox: Implement virDomainSendKey

[Dawid Zamirski]

Since the holdtime is not supported by VBOX SDK, it's being simulated
by sleeping before sending the key-up codes. The key-up codes are
auto-generated based on XT codeset rules (adding of 0x80 to key-down)
which results in the same behavior as for QEMU implementation.
---
 src/vbox/vbox_common.c | 107 +
 src/vbox/vbox_common.h |   1 +
 2 files changed, 108 insertions(+)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 0fb53aa..88bd226 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -33,6 +33,7 @@
 #include "virstring.h"
 #include "virfile.h"
 #include "virtime.h"
+#include "virkeycode.h"
 #include "snapshot_conf.h"
 #include "vbox_snapshot_conf.h"
 #include "fdstream.h"
@@ -7668,6 +7669,111 @@ vboxDomainHasManagedSaveImage(virDomainPtr dom, 
unsigned int flags)
 return ret;
 }
 
+static int
+vboxDomainSendKey(virDomainPtr dom,
+  unsigned int codeset,
+  unsigned int holdtime,
+  unsigned int *keycodes,
+  int nkeycodes,
+  unsigned int flags)
+{
+int ret = -1;
+vboxGlobalData *data = dom->conn->privateData;
+IConsole *console = NULL;
+vboxIIDUnion iid;
+IMachine *machine = NULL;
+IKeyboard *keyboard = NULL;
+PRInt32 *keyDownCodes = NULL;
+PRInt32 *keyUpCodes = NULL;
+PRUint32 codesStored = 0;
+nsresult rc;
+size_t i;
+int keycode;
+
+if (!data->vboxObj)
+return ret;
+
+virCheckFlags(0, -1);
+
+keyDownCodes = (PRInt32 *) keycodes;
+
+if (VIR_ALLOC_N(keyUpCodes, nkeycodes) < 0)
+return ret;
+
+/* translate keycodes to xt and generate keyup scancodes */
+for (i = 0; i < nkeycodes; i++) {
+if (codeset != VIR_KEYCODE_SET_XT) {
+keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_XT,
+   keyDownCodes[i]);
+if (keycode < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot translate keycode %u of %s codeset to"
+ " xt keycode"),
+ keyDownCodes[i],
+ virKeycodeSetTypeToString(codeset));
+goto cleanup;
+}
+keyDownCodes[i] = keycode;
+}
+
+keyUpCodes[i] = keyDownCodes[i] + 0x80;
+}
+
+if (openSessionForMachine(data, dom->uuid, &iid, &machine, false) < 0)
+goto cleanup;
+
+rc = gVBoxAPI.UISession.OpenExisting(data, &iid, machine);
+
+if (NS_FAILED(rc))
+goto cleanup;
+
+rc = gVBoxAPI.UISession.GetConsole(data->vboxSession, &console);
+
+if (NS_FAILED(rc) || !console)
+goto cleanup;
+
+rc = gVBoxAPI.UIConsole.GetKeyboard(console, &keyboard);
+
+if (NS_FAILED(rc))
+goto cleanup;
+
+rc = gVBoxAPI.UIKeyboard.PutScancodes(keyboard, nkeycodes, keyDownCodes,
+  &codesStored);
+
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("Unable to send keyboard scancodes"));
+goto cleanup;
+}
+
+/* since VBOX does not support holdtime, simulate it by sleeping and
+   then sending the release key scancodes */
+if (holdtime > 0) {
+usleep(holdtime * 1000);
+}
+
+rc = gVBoxAPI.UIKeyboard.PutScancodes(keyboard, nkeycodes, keyUpCodes,
+  &codesStored);
+
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("Unable to send keyboard scan codes"));
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(keyUpCodes);
+VBOX_RELEASE(keyboard);
+VBOX_RELEASE(console);
+gVBoxAPI.UISession.Close(data->vboxSession);
+VBOX_RELEASE(machine);
+vboxIIDUnalloc(&iid);
+
+return ret;
+}
+
 
 /**
  * Function Tables
@@ -7742,6 +7848,7 @@ virHypervisorDriver vboxCommonDriver = {
 .nodeGetFreePages = vboxNodeGetFreePages, /* 1.2.6 */
 .nodeAllocPages = vboxNodeAllocPages, /* 1.2.9 */
 .domainHasManagedSaveImage = vboxDomainHasManagedSaveImage, /* 1.2.13 */
+.domainSendKey = vboxDomainSendKey, /* 1.2.14 */
 };
 
 static void updateDriver(void)
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index d318921..ba19b1a 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -341,6 +341,7 @@ typedef nsISupports IHost;
 typedef nsISupports IHostNetworkInterface;
 typedef nsISupports IDHCPServer;
 typedef IMedium IHardDisk;
+typedef nsISupports IKeyboard;
 
 /* Macros for all vbox drivers. */
 
-- 
2.3.3

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

[libvirt] [RFC PATCH 1/2] vbox: Register IKeyboard with the unified API.

[Dawid Zamirski]

The IKeyboard COM object is needed to implement virDomainSendKey and is
available in all supported VBOX versions.
---
 src/vbox/vbox_tmpl.c  | 27 +++
 src/vbox/vbox_uniformed_api.h |  8 
 2 files changed, 35 insertions(+)

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 37ec8e1..22eecd4 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3437,6 +3437,12 @@ _consoleGetDisplay(IConsole *console, IDisplay **display)
 }
 
 static nsresult
+_consoleGetKeyboard(IConsole *console, IKeyboard **keyboard)
+{
+return console->vtbl->GetKeyboard(console, keyboard);
+}
+
+static nsresult
 _progressWaitForCompletion(IProgress *progress, PRInt32 timeout)
 {
 return progress->vtbl->WaitForCompletion(progress, timeout);
@@ -4599,6 +4605,20 @@ _hardDiskGetFormat(IHardDisk *hardDisk, PRUnichar 
**format)
 return hardDisk->vtbl->GetFormat(hardDisk, format);
 }
 
+static nsresult
+_keyboardPutScancode(IKeyboard *keyboard, PRInt32 scancode)
+{
+return keyboard->vtbl->PutScancode(keyboard, scancode);
+}
+
+static nsresult
+_keyboardPutScancodes(IKeyboard *keyboard, PRUint32 scancodesSize,
+  PRInt32 *scanCodes, PRUint32 *codesStored)
+{
+return keyboard->vtbl->PutScancodes(keyboard, scancodesSize, scanCodes,
+codesStored);
+}
+
 static bool _machineStateOnline(PRUint32 state)
 {
 return ((state >= MachineState_FirstOnline) &&
@@ -4757,6 +4777,7 @@ static vboxUniformedIConsole _UIConsole = {
 .TakeSnapshot = _consoleTakeSnapshot,
 .DeleteSnapshot = _consoleDeleteSnapshot,
 .GetDisplay = _consoleGetDisplay,
+.GetKeyboard = _consoleGetKeyboard,
 };
 
 static vboxUniformedIProgress _UIProgress = {
@@ -4951,6 +4972,11 @@ static vboxUniformedIHardDisk _UIHardDisk = {
 .GetFormat = _hardDiskGetFormat,
 };
 
+static vboxUniformedIKeyboard _UIKeyboard = {
+.PutScancode = _keyboardPutScancode,
+.PutScancodes = _keyboardPutScancodes,
+};
+
 static uniformedMachineStateChecker _machineStateChecker = {
 .Online = _machineStateOnline,
 .Inactive = _machineStateInactive,
@@ -5008,6 +5034,7 @@ void NAME(InstallUniformedAPI)(vboxUniformedAPI *pVBoxAPI)
 pVBoxAPI->UIHNInterface = _UIHNInterface;
 pVBoxAPI->UIDHCPServer = _UIDHCPServer;
 pVBoxAPI->UIHardDisk = _UIHardDisk;
+pVBoxAPI->UIKeyboard = _UIKeyboard;
 pVBoxAPI->machineStateChecker = _machineStateChecker;
 
 #if VBOX_API_VERSION <= 2002000 || VBOX_API_VERSION >= 400
diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h
index babc1e6..5d190ce 100644
--- a/src/vbox/vbox_uniformed_api.h
+++ b/src/vbox/vbox_uniformed_api.h
@@ -286,6 +286,7 @@ typedef struct {
  PRUnichar *description, IProgress **progress);
 nsresult (*DeleteSnapshot)(IConsole *console, vboxIIDUnion *iidu, 
IProgress **progress);
 nsresult (*GetDisplay)(IConsole *console, IDisplay **display);
+nsresult (*GetKeyboard)(IConsole *console, IKeyboard **keyboard);
 } vboxUniformedIConsole;
 
 /* Functions for IProgress */
@@ -534,6 +535,12 @@ typedef struct {
 } vboxUniformedIHardDisk;
 
 typedef struct {
+nsresult (*PutScancode)(IKeyboard *keyboard, PRInt32 scancode);
+nsresult (*PutScancodes)(IKeyboard *keyboard, PRUint32 scancodesSize,
+ PRInt32 *scanCodes, PRUint32 *codesStored);
+} vboxUniformedIKeyboard;
+
+typedef struct {
 bool (*Online)(PRUint32 state);
 bool (*Inactive)(PRUint32 state);
 bool (*NotStart)(PRUint32 state);
@@ -591,6 +598,7 @@ typedef struct {
 vboxUniformedIHNInterface UIHNInterface;
 vboxUniformedIDHCPServer UIDHCPServer;
 vboxUniformedIHardDisk UIHardDisk;
+vboxUniformedIKeyboard UIKeyboard;
 uniformedMachineStateChecker machineStateChecker;
 /* vbox API features */
 bool domainEventCallbacks;
-- 
2.3.3

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

Re: [libvirt] [PATCH 5/7] qemu: qemuDomainHotplugVcpus - separate out pin adjustment code

[John Ferlan]

On 03/19/2015 01:08 PM, John Ferlan wrote:
> Impending IOThread setting patches would copy the code anyway, so create
> and generalize the adding of pindef for the vcpu into its own API
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_driver.c | 104 
> +
>  1 file changed, 62 insertions(+), 42 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 673b95e..1fca43c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4669,6 +4669,63 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup,
>  return NULL;
>  }
>  
> +typedef int cgroupSetupFunc(virCgroupPtr cgroup,
> +virDomainPinDefPtr *pindef,
> +int npin,
> +int id);
> +
> +static int
> +qemuDomainHotplugAddPin(virBitmapPtr cpumask,
> +int index,
> +pid_t pid,
> +virDomainPinDefPtr **pindef_list,
> +size_t *npin,
> +cgroupSetupFunc func,
> +virCgroupPtr cgroup)
> +{
> +int ret = -1;
> +virDomainPinDefPtr pindef = NULL;
> +
> +/* vm->def->cputune.* arrays can't be NULL if
> + * vm->def->cpumask is not NULL.
> + */
> +if (VIR_ALLOC(pindef) < 0)
> +goto cleanup;
> +
> +if (!(pindef->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN))) {
> +VIR_FREE(pindef);
> +goto cleanup;
> +}
> +virBitmapCopy(pindef->cpumask, cpumask);
> +pindef->id = index;
> +if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) < 0) {
> +virBitmapFree(pindef->cpumask);
> +VIR_FREE(pindef);
> +goto cleanup;
> +}
> +
> +if (cgroup) {
> +if (func(cgroup, *pindef_list, *npin, index) < 0) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> +   _("failed to set cpuset.cpus in cgroup for id 
> %d"),
> +   index);
> +goto cleanup;
> +}
> +} else {
> +if (virProcessSetAffinity(pid, pindef->cpumask) < 0) {
> +virReportError(VIR_ERR_SYSTEM_ERROR,
> +   _("failed to set cpu affinity for id %d"),
> +   index);
> +goto cleanup;
> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +return ret;
> +}
> +
>  static int
>  qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup,
>cgroupNewFunc func,
> @@ -4795,51 +4852,14 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
>  
>  /* Inherit def->cpuset */
>  if (vm->def->cpumask) {
> -/* vm->def->cputune.vcpupin can't be NULL if
> - * vm->def->cpumask is not NULL.
> - */
> -virDomainPinDefPtr vcpupin = NULL;
> -
> -if (VIR_ALLOC(vcpupin) < 0)
> -goto cleanup;
> -
> -if (!(vcpupin->cpumask =
> -  virBitmapNew(VIR_DOMAIN_CPUMASK_LEN))) {
> -VIR_FREE(vcpupin);
> -goto cleanup;
> -}
> -virBitmapCopy(vcpupin->cpumask, vm->def->cpumask);
> -vcpupin->id = i;
> -if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin,
> -vm->def->cputune.nvcpupin, 
> vcpupin) < 0) {
> -virBitmapFree(vcpupin->cpumask);
> -VIR_FREE(vcpupin);
> +if (qemuDomainHotplugAddPin(vm->def->cpumask, i, cpupids[i],
> +&vm->def->cputune.vcpupin,
> +&vm->def->cputune.nvcpupin,
> +qemuSetupCgroupVcpuPin,
> +cgroup_vcpu) < 0)
>  ret = -1;
>  goto cleanup;

Ran my changes againt Coverity - looks like I missed something right
here - consider the following squashed in:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1fca43c..496a4af 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4856,9 +4856,10 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
 &vm->def->cputune.vcpupin,
 &vm->def->cputune.nvcpupin,
 qemuSetupCgroupVcpuPin,
-cgroup_vcpu) < 0)
+cgroup_vcpu) < 0) {
 ret = -1;
 goto cleanup;
+}
 }
 virCgroupFree(&cgroup_vcpu);




> -}
> -
> -if (cgroup_vcpu) {
> -if (qemuSetupCgroupVcpuPin(cgroup_vcpu,
> -

[libvirt] [PATCH] qemu_command.c: allow hostdev to be attached to PCIe bus

[Steven Walter]

Presently attaching a host device to the pcie-root controller fails, even if
the host device is actually a PCIe device.  With this change, the attachment
succeeds, QEMU is happy, and the guest correctly sees the host device as a
child of the PCIe root.
---
 src/qemu/qemu_command.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4d1590c..1de89e3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1377,6 +1377,13 @@ qemuCollectPCIAddress(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
  */
 flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
 break;
+
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+/* hostdev aren't hot-plugged, and can be put in either a
+ * PCI or PCIe slot
+ */
+flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
+break;
 }
 
 /* Ignore implicit controllers on slot 0:0:1.0:
-- 
2.1.0

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

Re: [libvirt] [Qemu-devel] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX

[Daniel P. Berrange]

On Thu, Mar 19, 2015 at 03:02:27PM -0300, Eduardo Habkost wrote:
> On Mon, Mar 16, 2015 at 10:24:51AM +, Daniel P. Berrange wrote:
> > On Fri, Mar 13, 2015 at 04:09:57PM -0300, Eduardo Habkost wrote:
> > > With the Intel microcode update that removed HLE and RTM, there will be
> > > different kinds of Haswell and Broadwell CPUs out there: some that still
> > > have the HLE and RTM features, and some that don't have the HLE and RTM
> > > features. On both cases people may be willing to use the pc-*-2.3
> > > machine-types.
> > > 
> > > So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU
> > > models, for hosts that have Haswell and Broadwell CPUs without TSX 
> > > support.
> > > 
> > > Signed-off-by: Eduardo Habkost 
> > 
> > The addition of Haswell-noTSX looks good to me.
> > 
> > I'm unclear on whether we truely need Broadwell-noTSX though. Did
> > Intel actually ship any Broadwell production silicon in which the
> > microcode disables this feature, or was it only a problem on
> > pre-production samples of Broadwell ? If the latter, I'd say we
> > don't need to have a Broadwell-noTSX model added. Perhaps Jun/Don
> > can confirm from Intel's side.
> 
> I've talked to Don and Jun, and they confirmed that a Broadwell-noTSX
> CPU model will be needed, too.
> 
> I see some Broadwell CPUs without TSX-NI on ark.intel.com, too, so the
> TSX errata wouldn't be the only reason for needing the -noTSX model.

Ok, your patch looks good.

  Reviewed-by: Daniel P. Berrange 

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] [Qemu-devel] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX

[Eduardo Habkost]

On Mon, Mar 16, 2015 at 10:24:51AM +, Daniel P. Berrange wrote:
> On Fri, Mar 13, 2015 at 04:09:57PM -0300, Eduardo Habkost wrote:
> > With the Intel microcode update that removed HLE and RTM, there will be
> > different kinds of Haswell and Broadwell CPUs out there: some that still
> > have the HLE and RTM features, and some that don't have the HLE and RTM
> > features. On both cases people may be willing to use the pc-*-2.3
> > machine-types.
> > 
> > So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU
> > models, for hosts that have Haswell and Broadwell CPUs without TSX support.
> > 
> > Signed-off-by: Eduardo Habkost 
> 
> The addition of Haswell-noTSX looks good to me.
> 
> I'm unclear on whether we truely need Broadwell-noTSX though. Did
> Intel actually ship any Broadwell production silicon in which the
> microcode disables this feature, or was it only a problem on
> pre-production samples of Broadwell ? If the latter, I'd say we
> don't need to have a Broadwell-noTSX model added. Perhaps Jun/Don
> can confirm from Intel's side.

I've talked to Don and Jun, and they confirmed that a Broadwell-noTSX
CPU model will be needed, too.

I see some Broadwell CPUs without TSX-NI on ark.intel.com, too, so the
TSX errata wouldn't be the only reason for needing the -noTSX model.

-- 
Eduardo

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

Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

[Richard Weinberger]

Am 19.03.2015 um 18:28 schrieb Daniel P. Berrange:
> On Thu, Mar 19, 2015 at 06:04:57PM +0100, Richard Weinberger wrote:
>> Am 19.03.2015 um 17:58 schrieb Daniel P. Berrange:
>>> On Thu, Mar 19, 2015 at 05:54:32PM +0100, Richard Weinberger wrote:
 Am 11.03.2015 um 10:36 schrieb Richard Weinberger:
> Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao:
 @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool 
 userns_enabled)
  bool bindOverReadonly;
  virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];

 +/* When enable userns but disable netns, kernel will
 + * forbid us doing a new fresh mount for sysfs.
 + * So we had to do a bind mount for sysfs instead.
 + */
 +if (userns_enabled && netns_disabled &&
 +STREQ(mnt->src, "sysfs")) {
 +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
 +goto cleanup;
 +}
>>>
>>> This is clearly broken and looks very untested to me.
>>>
>> It's broken now.
>> But when I submitted this patch last year, it's not.
>
> Are you sure?
> Just built libvirt v1.2.6-222-ga86b621, head is
> commit a86b6215a74b1feb2667204e214fbfd2f7decc5c
> Author: Chen Hanxiao 
> Date:   Mon Jul 14 18:01:51 2014 +0800
>
> LXC: create a bind mount for sysfs when enable userns but disable 
> netns
>
> /sys is still an empty directory but as at this time (most likely due to 
> another bug)
> libvirt was able to create /sys/fs/cgroup and mounted groups there.
> But no sysfs at all is at /sys.
>
> I mean, how is this supposed to work? You bind mount /sys over /sys...

 Any further comments on that?
>>>
>>> It just looks impossible for it to work in this way
>>
>> That's also my impression.
>>
>> Therefore containers without their own network namespace currently don't work
>> and have never worked as expected.
> 
> No, it is only a problem if userns is used. If userns is not used then
> they do work

Agreed.

>> Shall we revert commit a86b6215a74b and try to bind mount
>> before the pivot_root()?
> 
> Not sure if that works with userns is active either.

Fact is that commit a86b6215a74 is broken.
We could also refuse to create container with userns enabled but netns 
disabled...

Thanks,
//richard

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

Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

[Daniel P. Berrange]

On Thu, Mar 19, 2015 at 06:04:57PM +0100, Richard Weinberger wrote:
> Am 19.03.2015 um 17:58 schrieb Daniel P. Berrange:
> > On Thu, Mar 19, 2015 at 05:54:32PM +0100, Richard Weinberger wrote:
> >> Am 11.03.2015 um 10:36 schrieb Richard Weinberger:
> >>> Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao:
> >> @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool 
> >> userns_enabled)
> >>  bool bindOverReadonly;
> >>  virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
> >>
> >> +/* When enable userns but disable netns, kernel will
> >> + * forbid us doing a new fresh mount for sysfs.
> >> + * So we had to do a bind mount for sysfs instead.
> >> + */
> >> +if (userns_enabled && netns_disabled &&
> >> +STREQ(mnt->src, "sysfs")) {
> >> +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
> >> +goto cleanup;
> >> +}
> >
> > This is clearly broken and looks very untested to me.
> >
>  It's broken now.
>  But when I submitted this patch last year, it's not.
> >>>
> >>> Are you sure?
> >>> Just built libvirt v1.2.6-222-ga86b621, head is
> >>> commit a86b6215a74b1feb2667204e214fbfd2f7decc5c
> >>> Author: Chen Hanxiao 
> >>> Date:   Mon Jul 14 18:01:51 2014 +0800
> >>>
> >>> LXC: create a bind mount for sysfs when enable userns but disable 
> >>> netns
> >>>
> >>> /sys is still an empty directory but as at this time (most likely due to 
> >>> another bug)
> >>> libvirt was able to create /sys/fs/cgroup and mounted groups there.
> >>> But no sysfs at all is at /sys.
> >>>
> >>> I mean, how is this supposed to work? You bind mount /sys over /sys...
> >>
> >> Any further comments on that?
> > 
> > It just looks impossible for it to work in this way
> 
> That's also my impression.
> 
> Therefore containers without their own network namespace currently don't work
> and have never worked as expected.

No, it is only a problem if userns is used. If userns is not used then
they do work

> Shall we revert commit a86b6215a74b and try to bind mount
> before the pivot_root()?

Not sure if that works with userns is active either.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [PATCH 3/7] qemu: qemuDomainHotplugVcpus - separate out the add cgroup

[John Ferlan]

Impending IOThread setting patches would copy the code anyway, so create
and generalize the add the vcpu to a cgroup into its own API.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c | 69 +++---
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d9217b..b7ddca3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4630,9 +4630,49 @@ static void qemuProcessEventHandler(void *data, void 
*opaque)
 VIR_FREE(processEvent);
 }
 
-static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
-  virDomainObjPtr vm,
-  unsigned int nvcpus)
+typedef int cgroupNewFunc(virCgroupPtr domain,
+  int id,
+  bool create,
+  virCgroupPtr *group);
+
+static virCgroupPtr
+qemuDomainHotplugAddCgroup(virCgroupPtr cgroup,
+   cgroupNewFunc func,
+   int index,
+   char *mem_mask,
+   pid_t pid)
+{
+virCgroupPtr new_cgroup = NULL;
+int rv = -1;
+
+/* Create cgroup */
+if (func(cgroup, index, true, &new_cgroup) < 0)
+return NULL;
+
+if (mem_mask && virCgroupSetCpusetMems(new_cgroup, mem_mask) < 0)
+goto error;
+
+/* Add pid/thread to the cgroup */
+rv = virCgroupAddTask(new_cgroup, pid);
+if (rv < 0) {
+virReportSystemError(-rv,
+ _("unable to add id %d task %d to cgroup"),
+ index, pid);
+virCgroupRemove(new_cgroup);
+goto error;
+}
+
+return new_cgroup;
+
+ error:
+virCgroupFree(&new_cgroup);
+return NULL;
+}
+
+static int
+qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   unsigned int nvcpus)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 size_t i;
@@ -4721,25 +4761,12 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr 
driver,
 if (nvcpus > oldvcpus) {
 for (i = oldvcpus; i < nvcpus; i++) {
 if (priv->cgroup) {
-int rv = -1;
-/* Create cgroup for the onlined vcpu */
-if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0)
+cgroup_vcpu = qemuDomainHotplugAddCgroup(priv->cgroup,
+ virCgroupNewVcpu,
+ i, mem_mask,
+ cpupids[i]);
+if (!cgroup_vcpu)
 goto cleanup;
-
-if (mem_mask &&
-virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0)
-goto cleanup;
-
-/* Add vcpu thread to the cgroup */
-rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]);
-if (rv < 0) {
-virReportSystemError(-rv,
- _("unable to add vcpu %zu task %d to 
cgroup"),
- i, cpupids[i]);
-virCgroupRemove(cgroup_vcpu);
-goto cleanup;
-}
-
 }
 
 /* Inherit def->cpuset */
-- 
2.1.0

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

[libvirt] [PATCH 6/7] qemu: Add support to set IOThreads count

[John Ferlan]

Add qemuDomainSetIOThreads in order to adjust the live and config IOThread
values for the domain similar to the qemuDomainSetVcpus implementation.

The qemuDomainHotplugIOThread will more or less mirror qemuDomainHotplugVcpus
including usage of the recently extracted API's to manage the cgroup and
adding pinning if the domain has a cpuset.

Unlike the Vcpu code, IOThreads are implemented by qmp objects. Forcing the
HotplugIOThread code to add/delete one at a time to/from the end of the list
rather than attempting to batch add/delete more than one and attempting to
manage/determine which failed leaving potential holes unsequenced id numbers.

Signed-off-by: John Ferlan 
---
 src/conf/domain_audit.c  |   9 ++
 src/conf/domain_audit.h  |   6 +
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_driver.c   | 331 +++
 4 files changed, 347 insertions(+)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 159ebf5..92fcdd3 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -790,6 +790,15 @@ virDomainAuditVcpu(virDomainObjPtr vm,
 return virDomainAuditResource(vm, "vcpu", oldvcpu, newvcpu, reason, 
success);
 }
 
+void
+virDomainAuditIOThread(virDomainObjPtr vm,
+   unsigned int oldiothread, unsigned int newiothread,
+   const char *reason, bool success)
+{
+return virDomainAuditResource(vm, "iothread", oldiothread, newiothread,
+  reason, success);
+}
+
 static void
 virDomainAuditLifecycle(virDomainObjPtr vm, const char *op,
 const char *reason, bool success)
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h
index 4c1ef90..97dadca 100644
--- a/src/conf/domain_audit.h
+++ b/src/conf/domain_audit.h
@@ -102,6 +102,12 @@ void virDomainAuditVcpu(virDomainObjPtr vm,
 const char *reason,
 bool success)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
+void virDomainAuditIOThread(virDomainObjPtr vm,
+unsigned int oldiothread,
+unsigned int newiothread,
+const char *reason,
+bool success)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
 void virDomainAuditSecurityLabel(virDomainObjPtr vm,
  bool success)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ca3520d..0d09c23 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -119,6 +119,7 @@ virDomainAuditDisk;
 virDomainAuditFS;
 virDomainAuditHostdev;
 virDomainAuditInit;
+virDomainAuditIOThread;
 virDomainAuditMemory;
 virDomainAuditNet;
 virDomainAuditNetDevice;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1fca43c..dbd7ff5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6182,6 +6182,336 @@ qemuDomainPinIOThread(virDomainPtr dom,
 return ret;
 }
 
+static int
+qemuDomainHotplugIOThread(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  unsigned int req_niothreads)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+char *alias = NULL;
+size_t i;
+int rc = -1;
+int ret = -1;
+unsigned int old_niothreads = vm->def->iothreads;
+int new_niothreads = 0;
+qemuMonitorIOThreadsInfoPtr *new_iothreads = NULL;
+virCgroupPtr cgroup_iothread = NULL;
+char *mem_mask = NULL;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("cannot change IOThreads for an inactive domain"));
+goto exit_monitor;
+}
+
+/* Only allow to hot add or remove one IOThread at a time forcing
+ * the caller to "do the right thing" especially with respect to
+ * deletion since we're dealing with unsigned int math. Getting
+ * this error probably proves the caller used some bad math.
+ */
+if (req_niothreads > old_niothreads + 1 ||
+(old_niothreads && req_niothreads < old_niothreads - 1)) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("only allowed to hot add or remove one IOThread "
+ "at a time for the domain, current count is: %d"),
+   old_niothreads);
+goto exit_monitor;
+}
+
+if (req_niothreads > old_niothreads) {
+/* Adding to end - new alias is req_niothreads */
+if (virAsprintf(&alias, "iothread%u", req_niothreads) < 0)
+goto exit_monitor;
+
+rc = qemuMonitorAddObject(priv->mon, "iothread", alias, NULL);
+} else {
+/* Removing from end - alias is current old_niothreads */
+if (virAsprintf(&alias, "iothread%u", old_niothreads) < 0)
+goto exit_monitor;
+
+rc = qemuMonitorDelObject(priv->mon, alias);
+}
+
+   

[libvirt] [PATCH 0/7] Implement Set IOThreads Command

[John Ferlan]

These patches will implement the Set IOThreads command to complete
the IOThreads family of calls/settings.  There are 3 patches in the
middle (3, 4, & 5) which are just refactoring the SetVcpus code so
that the SetIOThreads code can make use of the same functionality
rather than cut-n-paste


John Ferlan (7):
  Implement public API for virDomainIOThreadsSet
  remote: Plumbing for virDomainSetIOThreads
  qemu: qemuDomainHotplugVcpus - separate out the add cgroup
  qemu: qemuDomainHotplugVcpus - separate out the del cgroup and pin
  qemu: qemuDomainHotplugVcpus - separate out pin adjustment code
  qemu: Add support to set IOThreads count
  virsh: Add setiothreads command

 include/libvirt/libvirt-domain.h |   3 +
 src/conf/domain_audit.c  |   9 +
 src/conf/domain_audit.h  |   6 +
 src/driver-hypervisor.h  |   6 +
 src/libvirt-domain.c |  60 +
 src/libvirt_private.syms |   1 +
 src/libvirt_public.syms  |   1 +
 src/qemu/qemu_driver.c   | 544 +--
 src/remote/remote_driver.c   |   1 +
 src/remote/remote_protocol.x |  16 +-
 src/remote_protocol-structs  |   6 +
 tools/virsh-domain.c |  82 ++
 tools/virsh.pod  |  17 ++
 13 files changed, 676 insertions(+), 76 deletions(-)

-- 
2.1.0

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

[libvirt] [PATCH 5/7] qemu: qemuDomainHotplugVcpus - separate out pin adjustment code

[John Ferlan]

Impending IOThread setting patches would copy the code anyway, so create
and generalize the adding of pindef for the vcpu into its own API

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c | 104 +
 1 file changed, 62 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 673b95e..1fca43c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4669,6 +4669,63 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup,
 return NULL;
 }
 
+typedef int cgroupSetupFunc(virCgroupPtr cgroup,
+virDomainPinDefPtr *pindef,
+int npin,
+int id);
+
+static int
+qemuDomainHotplugAddPin(virBitmapPtr cpumask,
+int index,
+pid_t pid,
+virDomainPinDefPtr **pindef_list,
+size_t *npin,
+cgroupSetupFunc func,
+virCgroupPtr cgroup)
+{
+int ret = -1;
+virDomainPinDefPtr pindef = NULL;
+
+/* vm->def->cputune.* arrays can't be NULL if
+ * vm->def->cpumask is not NULL.
+ */
+if (VIR_ALLOC(pindef) < 0)
+goto cleanup;
+
+if (!(pindef->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN))) {
+VIR_FREE(pindef);
+goto cleanup;
+}
+virBitmapCopy(pindef->cpumask, cpumask);
+pindef->id = index;
+if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) < 0) {
+virBitmapFree(pindef->cpumask);
+VIR_FREE(pindef);
+goto cleanup;
+}
+
+if (cgroup) {
+if (func(cgroup, *pindef_list, *npin, index) < 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("failed to set cpuset.cpus in cgroup for id %d"),
+   index);
+goto cleanup;
+}
+} else {
+if (virProcessSetAffinity(pid, pindef->cpumask) < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("failed to set cpu affinity for id %d"),
+   index);
+goto cleanup;
+}
+}
+
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
 static int
 qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup,
   cgroupNewFunc func,
@@ -4795,51 +4852,14 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
 
 /* Inherit def->cpuset */
 if (vm->def->cpumask) {
-/* vm->def->cputune.vcpupin can't be NULL if
- * vm->def->cpumask is not NULL.
- */
-virDomainPinDefPtr vcpupin = NULL;
-
-if (VIR_ALLOC(vcpupin) < 0)
-goto cleanup;
-
-if (!(vcpupin->cpumask =
-  virBitmapNew(VIR_DOMAIN_CPUMASK_LEN))) {
-VIR_FREE(vcpupin);
-goto cleanup;
-}
-virBitmapCopy(vcpupin->cpumask, vm->def->cpumask);
-vcpupin->id = i;
-if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin,
-vm->def->cputune.nvcpupin, 
vcpupin) < 0) {
-virBitmapFree(vcpupin->cpumask);
-VIR_FREE(vcpupin);
+if (qemuDomainHotplugAddPin(vm->def->cpumask, i, cpupids[i],
+&vm->def->cputune.vcpupin,
+&vm->def->cputune.nvcpupin,
+qemuSetupCgroupVcpuPin,
+cgroup_vcpu) < 0)
 ret = -1;
 goto cleanup;
-}
-
-if (cgroup_vcpu) {
-if (qemuSetupCgroupVcpuPin(cgroup_vcpu,
-   vm->def->cputune.vcpupin,
-   vm->def->cputune.nvcpupin, i) < 
0) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("failed to set cpuset.cpus in cgroup"
- " for vcpu %zu"), i);
-ret = -1;
-goto cleanup;
-}
-} else {
-if (virProcessSetAffinity(cpupids[i],
-  vcpupin->cpumask) < 0) {
-virReportError(VIR_ERR_SYSTEM_ERROR,
-   _("failed to set cpu affinity for vcpu 
%zu"),
-   i);
-ret = -1;
-goto cleanup;
-}
-}
 }
-
 virCgroupFree(&cgroup_vcpu);
 
 if (qemuProcessSetSchedParams(i, cpupids[i],
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat

[libvirt] [PATCH 2/7] remote: Plumbing for virDomainSetIOThreads

[John Ferlan]

Add the necessary remote plumbing

Signed-off-by: John Ferlan 
---
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 16 +++-
 src/remote_protocol-structs  |  6 ++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e69f235..fed001c 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8210,6 +8210,7 @@ static virHypervisorDriver hypervisor_driver = {
 .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */
 .domainGetIOThreadsInfo = remoteDomainGetIOThreadsInfo, /* 1.2.14 */
 .domainPinIOThread = remoteDomainPinIOThread, /* 1.2.14 */
+.domainSetIOThreads = remoteDomainSetIOThreads, /* 1.2.14 */
 .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */
 .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 
*/
 .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index eb862e1..19282ed 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1212,6 +1212,12 @@ struct remote_domain_pin_iothread_args {
 unsigned int flags;
 };
 
+struct remote_domain_set_iothreads_args {
+remote_nonnull_domain dom;
+unsigned int niothreads;
+unsigned int flags;
+};
+
 struct remote_domain_get_security_label_args {
 remote_nonnull_domain dom;
 };
@@ -5643,5 +5649,13 @@ enum remote_procedure {
  * @generate: none
  * @acl: domain:read
  */
-REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353
+REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353,
+
+/**
+ * @generate: both
+ * @acl: domain:write
+ * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE
+ * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG
+ */
+REMOTE_PROC_DOMAIN_SET_IOTHREADS = 354
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index b3e2e40..e046076 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -834,6 +834,11 @@ struct remote_domain_pin_iothread_args {
 } cpumap;
 u_int  flags;
 };
+struct remote_domain_set_iothreads_args {
+remote_nonnull_domain  dom;
+u_int  niothreads;
+u_int  flags;
+};
 struct remote_domain_get_security_label_args {
 remote_nonnull_domain  dom;
 };
@@ -3017,4 +3022,5 @@ enum remote_procedure {
 REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351,
 REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352,
 REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353,
+REMOTE_PROC_DOMAIN_SET_IOTHREADS = 354,
 };
-- 
2.1.0

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

[libvirt] [PATCH 4/7] qemu: qemuDomainHotplugVcpus - separate out the del cgroup and pin

[John Ferlan]

Impending IOThread setting patches would copy the code anyway, so create
and generalize a delete cgroup and pindef for the vcpu into its own API.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c | 40 
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b7ddca3..673b95e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4670,6 +4670,30 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup,
 }
 
 static int
+qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup,
+  cgroupNewFunc func,
+  int index,
+  virDomainPinDefPtr **pindef_list,
+  size_t *npin)
+{
+virCgroupPtr new_cgroup = NULL;
+
+if (cgroup) {
+if (func(cgroup, index, false, &new_cgroup) < 0)
+return -1;
+
+/* Remove the offlined cgroup */
+virCgroupRemove(new_cgroup);
+virCgroupFree(&new_cgroup);
+}
+
+/* Free pindef setting */
+virDomainPinDel(pindef_list, npin, index);
+
+return 0;
+}
+
+static int
 qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
virDomainObjPtr vm,
unsigned int nvcpus)
@@ -4825,19 +4849,11 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
 }
 } else {
 for (i = oldvcpus - 1; i >= nvcpus; i--) {
-if (priv->cgroup) {
-if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0)
-goto cleanup;
-
-/* Remove cgroup for the offlined vcpu */
-virCgroupRemove(cgroup_vcpu);
-virCgroupFree(&cgroup_vcpu);
-}
+if (qemuDomainHotplugDelCgroupPin(priv->cgroup, virCgroupNewVcpu, 
i,
+  &vm->def->cputune.vcpupin,
+  &vm->def->cputune.nvcpupin) < 0)
+goto cleanup;
 
-/* Free vcpupin setting */
-virDomainPinDel(&vm->def->cputune.vcpupin,
-&vm->def->cputune.nvcpupin,
-i);
 }
 }
 
-- 
2.1.0

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

[libvirt] [PATCH 1/7] Implement public API for virDomainIOThreadsSet

[John Ferlan]

Add virDomainIOThreadsSet to allow setting the number of IOThreads for
a domain.

The API supports updating both the --live domain and the --config data.

Signed-off-by: John Ferlan 
---
 include/libvirt/libvirt-domain.h |  3 ++
 src/driver-hypervisor.h  |  6 
 src/libvirt-domain.c | 60 
 src/libvirt_public.syms  |  1 +
 4 files changed, 70 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 5d1d868..d7ce6f8 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1615,6 +1615,9 @@ int  virDomainPinIOThread(virDomainPtr 
domain,
   unsigned char *cpumap,
   int maplen,
   unsigned int flags);
+int  virDomainSetIOThreads(virDomainPtr domain,
+   unsigned int niothreads,
+   unsigned int flags);
 
 /**
  * VIR_USE_CPU:
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 3f9bf02..2e4104b 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -393,6 +393,11 @@ typedef int
unsigned int flags);
 
 typedef int
+(*virDrvDomainSetIOThreads)(virDomainPtr domain,
+unsigned int iothreads,
+unsigned int flags);
+
+typedef int
 (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
 virSecurityLabelPtr seclabel);
 
@@ -1273,6 +1278,7 @@ struct _virHypervisorDriver {
 virDrvDomainGetMaxVcpus domainGetMaxVcpus;
 virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo;
 virDrvDomainPinIOThread domainPinIOThread;
+virDrvDomainSetIOThreads domainSetIOThreads;
 virDrvDomainGetSecurityLabel domainGetSecurityLabel;
 virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
 virDrvNodeGetSecurityModel nodeGetSecurityModel;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 0bd9274..7176301 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8042,6 +8042,66 @@ virDomainPinIOThread(virDomainPtr domain,
 
 
 /**
+ * virDomainSetIOThreads:
+ * @domain: pointer to domain object
+ * @niothreads: the new number of IOThreads for this domain
+ * @flags: bitwise-OR of virDomainModificationImpact
+ *
+ * Dynamically change the number of IOThreads used by the domain.
+ * Note that this call faiy fail if the underlying virtualization hypervisor
+ * does not support it or if growing the number is arbitrarily limited.
+ * This function may require privileged access to the hypervisor.
+ *
+ * @flags may include VIR_DOMAIN_AFFECT_LIVE to affect a running
+ * domain (which may fail if domain is not active), or
+ * VIR_DOMAIN_AFFECT_CONFIG to affect the next boot via the XML
+ * description of the domain.  Both flags may be set.
+ * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT),
+ * then an inactive domain modifies persistent setup, while an active domain
+ * is hypervisor-dependent on whether just live or both live and persistent
+ * state is changed.
+ *
+ * Not all hypervisors can support all flag combinations.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+int
+virDomainSetIOThreads(virDomainPtr domain,
+  unsigned int niothreads,
+  unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "niothreads=%u, flags=%x", niothreads, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+virCheckReadOnlyGoto(domain->conn->flags, error);
+
+if ((unsigned short) niothreads != niothreads) {
+virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), niothreads);
+goto error;
+}
+conn = domain->conn;
+
+if (conn->driver->domainSetIOThreads) {
+int ret;
+ret = conn->driver->domainSetIOThreads(domain, niothreads, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return -1;
+}
+
+
+/**
  * virDomainGetSecurityLabel:
  * @domain: a domain object
  * @seclabel: pointer to a virSecurityLabel structure
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index e4cf7ed..fdcbd6f 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -702,6 +702,7 @@ LIBVIRT_1.2.14 {
 virDomainPinIOThread;
 virDomainInterfaceAddresses;
 virDomainInterfaceFree;
+virDomainSetIOThreads;
 } LIBVIRT_1.2.12;
 
 #  define new API here using predicted next version number 
-- 
2.1.0

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

[libvirt] [PATCH 7/7] virsh: Add setiothreads command

[John Ferlan]

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

Add command to allow adding and removing IOThreads from the domain including
the configuration and live domain.

$ virsh setiothreads --help
  NAME
setiothreads - change number of IOThreads

  SYNOPSIS
setiothreads   [--config] [--live] [--current]

  DESCRIPTION
Change the number of IOThreads in the guest domain.

  OPTIONS
[--domain]   domain name, id or uuid
[--count]   number of IOThreads
--config affect next boot
--live   affect running domain
--currentaffect current domain

Assuming a domain with multiple IOThreads assigned to various threads and
trying to remove an IOThread assigned to a disk resource results in an
error on removal attempt:

$ virsh iothreadsinfo $dom
 IOThread ID CPU Affinity
 ---
  1   2
  2   3
  3   0-3

$ virsh setiothreads $dom 1
error: invalid argument: cannot remove IOThread 2 since it is being used by 
disk path '/home/vm-images/iothr-vol1'

Adding an IOThread to a domain and then removing it:

$ virsh iothreadsinfo $dom
No IOThreads found for the domain

$ virsh setiothreads $dom 1
$ virsh iothreadsinfo $dom
 IOThread ID CPU Affinity
 ---
  1   0-3

$ virsh setiothreads $dom 0
$ virsh iothreadsinfo $dom
No IOThreads found for the domain
$

Invalid number of IOThreads provided:

$ virsh setiothreads $dom -1
error: Invalid number of IOThreads

$

Signed-off-by: John Ferlan 
---
 tools/virsh-domain.c | 82 
 tools/virsh.pod  | 17 +++
 2 files changed, 99 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1d8225c..69d2a94 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6978,6 +6978,82 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "setiothreads" command
+ */
+static const vshCmdInfo info_setiothreads[] = {
+{.name = "help",
+ .data = N_("change number of IOThreads")
+},
+{.name = "desc",
+ .data = N_("Change the number of IOThreads in the guest domain.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_setiothreads[] = {
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("domain name, id or uuid")
+},
+{.name = "count",
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("number of IOThreads")
+},
+{.name = "config",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect next boot")
+},
+{.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+},
+{.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+},
+{.name = NULL}
+};
+
+static bool
+cmdSetIOThreads(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+int count = 0;
+bool ret = false;
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+bool current = vshCommandOptBool(cmd, "current");
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptInt(cmd, "count", &count) < 0 || count < 0) {
+vshError(ctl, "%s", _("Invalid number of IOThreads"));
+goto cleanup;
+}
+
+if (virDomainSetIOThreads(dom, count, flags) < 0)
+goto cleanup;
+
+ret = true;
+
+ cleanup:
+virDomainFree(dom);
+return ret;
+}
+
+/*
  * "cpu-compare" command
  */
 static const vshCmdInfo info_cpu_compare[] = {
@@ -12903,6 +12979,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_iothreadpin,
  .flags = 0
 },
+{.name = "setiothreads",
+ .handler = cmdSetIOThreads,
+ .opts = opts_setiothreads,
+ .info = info_setiothreads,
+ .flags = 0
+},
 {.name = "send-key",
  .handler = cmdSendKey,
  .opts = opts_send_key,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 8262a45..3376e49 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1416,6 +1416,23 @@ If no flag is specified, behavior is different depending 
on hypervisor.
 B: The expression is sequentially evaluated, so "0-15,^8" is
 identical to "9-14,0-7,15" but not identical to "^8,0-15".
 
+=item B I I
+[[I<--config>] [I<--live>] | [I<--current>]]
+
+Change the number of IOThreads in a guest domain. The I value may be
+limited by host or hypervisor. A value of zero removes all IOThreads from
+the guest domain. If an IOThread is currently assigned to a disk resource
+such as via the B command, then an attempt to remove the
+IOThread will fai

Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

[Richard Weinberger]

Am 19.03.2015 um 17:58 schrieb Daniel P. Berrange:
> On Thu, Mar 19, 2015 at 05:54:32PM +0100, Richard Weinberger wrote:
>> Am 11.03.2015 um 10:36 schrieb Richard Weinberger:
>>> Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao:
>> @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool 
>> userns_enabled)
>>  bool bindOverReadonly;
>>  virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
>>
>> +/* When enable userns but disable netns, kernel will
>> + * forbid us doing a new fresh mount for sysfs.
>> + * So we had to do a bind mount for sysfs instead.
>> + */
>> +if (userns_enabled && netns_disabled &&
>> +STREQ(mnt->src, "sysfs")) {
>> +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
>> +goto cleanup;
>> +}
>
> This is clearly broken and looks very untested to me.
>
 It's broken now.
 But when I submitted this patch last year, it's not.
>>>
>>> Are you sure?
>>> Just built libvirt v1.2.6-222-ga86b621, head is
>>> commit a86b6215a74b1feb2667204e214fbfd2f7decc5c
>>> Author: Chen Hanxiao 
>>> Date:   Mon Jul 14 18:01:51 2014 +0800
>>>
>>> LXC: create a bind mount for sysfs when enable userns but disable netns
>>>
>>> /sys is still an empty directory but as at this time (most likely due to 
>>> another bug)
>>> libvirt was able to create /sys/fs/cgroup and mounted groups there.
>>> But no sysfs at all is at /sys.
>>>
>>> I mean, how is this supposed to work? You bind mount /sys over /sys...
>>
>> Any further comments on that?
> 
> It just looks impossible for it to work in this way

That's also my impression.

Therefore containers without their own network namespace currently don't work
and have never worked as expected.

Shall we revert commit a86b6215a74b and try to bind mount
before the pivot_root()?

Thanks,
//richard

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

Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

[Daniel P. Berrange]

On Thu, Mar 19, 2015 at 05:54:32PM +0100, Richard Weinberger wrote:
> Am 11.03.2015 um 10:36 schrieb Richard Weinberger:
> > Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao:
>  @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool 
>  userns_enabled)
>   bool bindOverReadonly;
>   virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
> 
>  +/* When enable userns but disable netns, kernel will
>  + * forbid us doing a new fresh mount for sysfs.
>  + * So we had to do a bind mount for sysfs instead.
>  + */
>  +if (userns_enabled && netns_disabled &&
>  +STREQ(mnt->src, "sysfs")) {
>  +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
>  +goto cleanup;
>  +}
> >>>
> >>> This is clearly broken and looks very untested to me.
> >>>
> >> It's broken now.
> >> But when I submitted this patch last year, it's not.
> > 
> > Are you sure?
> > Just built libvirt v1.2.6-222-ga86b621, head is
> > commit a86b6215a74b1feb2667204e214fbfd2f7decc5c
> > Author: Chen Hanxiao 
> > Date:   Mon Jul 14 18:01:51 2014 +0800
> > 
> > LXC: create a bind mount for sysfs when enable userns but disable netns
> > 
> > /sys is still an empty directory but as at this time (most likely due to 
> > another bug)
> > libvirt was able to create /sys/fs/cgroup and mounted groups there.
> > But no sysfs at all is at /sys.
> > 
> > I mean, how is this supposed to work? You bind mount /sys over /sys...
> 
> Any further comments on that?

It just looks impossible for it to work in this way

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

[Richard Weinberger]

Am 11.03.2015 um 10:36 schrieb Richard Weinberger:
> Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao:
 @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool 
 userns_enabled)
  bool bindOverReadonly;
  virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];

 +/* When enable userns but disable netns, kernel will
 + * forbid us doing a new fresh mount for sysfs.
 + * So we had to do a bind mount for sysfs instead.
 + */
 +if (userns_enabled && netns_disabled &&
 +STREQ(mnt->src, "sysfs")) {
 +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
 +goto cleanup;
 +}
>>>
>>> This is clearly broken and looks very untested to me.
>>>
>> It's broken now.
>> But when I submitted this patch last year, it's not.
> 
> Are you sure?
> Just built libvirt v1.2.6-222-ga86b621, head is
> commit a86b6215a74b1feb2667204e214fbfd2f7decc5c
> Author: Chen Hanxiao 
> Date:   Mon Jul 14 18:01:51 2014 +0800
> 
> LXC: create a bind mount for sysfs when enable userns but disable netns
> 
> /sys is still an empty directory but as at this time (most likely due to 
> another bug)
> libvirt was able to create /sys/fs/cgroup and mounted groups there.
> But no sysfs at all is at /sys.
> 
> I mean, how is this supposed to work? You bind mount /sys over /sys...

Any further comments on that?

Thanks,
//richard

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

Re: [libvirt] [PATCH] libxl: use xenlight pkgconfig file if present

[Michal Privoznik]

On 18.03.2015 23:25, Jim Fehlig wrote:
> xen.git commit babeca32 added a pkgconfig file for libxenlight,
> allowing libxl apps to determine the location of Xen binaries
> such as firmware blobs, device emulator, etc.
> 
> This patch adds support for xenlight.pc in the libxl driver, falling
> back to the previous configure logic if not found.  It introduces
> LIBXL_FIRMWARE_DIR and LIBXL_EXECBIN_DIR to define the firmware and
> libexec_bin locations.  If xenlight.pc does not exist, the defines
> are set to the current hardcoded paths.  The capabilities'
>  and  elements are updated to use the paths.
> 
> Signed-off-by: Jim Fehlig 
> ---
>  configure.ac   | 47 +--
>  src/libxl/libxl_conf.c |  7 ++-
>  src/libxl/libxl_conf.h |  8 
>  3 files changed, 43 insertions(+), 19 deletions(-)

ACK

Michal

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

Re: [libvirt] [PATCH] parallels: fix libvirt crash if parallelsNetworkClose fails

[Maxim Nestratov]

Right. I sent this by a mistake and then asked to ignore it.
Thank you. 

Maxim

> -Original Message-
> From: Michal Privoznik [mailto:mpriv...@redhat.com]
> Sent: Thursday, March 19, 2015 6:41 PM
> To: Maxim Nestratov; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH] parallels: fix libvirt crash if 
> parallelsNetworkClose fails
> 
> On 19.03.2015 15:36, Maxim Nestratov wrote:
> > If, by any reason, parallelsNetworkClose fails it dereferences
> > newly allocated privconn->networks via virObjectUnref, which in
> > turn deallocates its memory.
> > Subsequent call of parallelsNetworkClose calls virObjectUnref
> > that leads to double memory free. To prevent this we should zero
> > privconn->networks to make all subsequent virObjectUnref be safe.
> >
> > Signed-off-by: Maxim Nestratov 
> > ---
> >  src/parallels/parallels_network.c |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/parallels/parallels_network.c 
> > b/src/parallels/parallels_network.c
> > index 8cc0582..8caad4a 100644
> > --- a/src/parallels/parallels_network.c
> > +++ b/src/parallels/parallels_network.c
> > @@ -348,6 +348,7 @@ parallelsNetworkOpen(virConnectPtr conn,
> >  return VIR_DRV_OPEN_SUCCESS;
> >   error:
> >  virObjectUnref(privconn->networks);
> > +privconn->networks = NULL;
> >  return VIR_DRV_OPEN_DECLINED;
> >  }
> >
> >
> 
> This patch is to be ignored since I've pushed the other one:
> 
>   https://www.redhat.com/archives/libvir-list/2015-March/msg01002.html
> 
> Michal

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

Re: [libvirt] qemu_migration: Precreate missing storage breaks with network drives

[Nick Bartos]

That does sound like the best option then.  Would you mind proposing a nova
patch?

On Thu, Mar 19, 2015 at 8:37 AM, Michal Privoznik 
wrote:

> On 19.03.2015 16:29, Nick Bartos wrote:
> > That sounds like a reasonable option.  How far back is the shared disk
> > type supported?  As of Icehouse (the oldest supported version I
> > believe), the minimum supported libvirt version is 0.9.6.  It would be
> > simpler if it could be a simple change that doesn't have to be if/else'd
> > for different libvirt versions.
> >
>
> The  element was added back in 0.4.2 release (we're talking
> about mid 2008). So I wouldn't fear of it. Here's an example XML taken
> from one of our unit tests to show you how to use it:
>
> 
>   
>   
>   
>   
>   XYZXYZXYZYXXYZYZYXYZY
>   
> 
>
>
> Michal
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] parallels: implement .domainGetMaxMemory

[Dmitry Guryanov]

Since we haven't implemented balloon parameters tuning
we can just return amount of memory in this function.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_driver.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 391e927..d13a4e2 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1089,6 +1089,24 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, 
unsigned int flags)
 return ret;
 }
 
+static unsigned long long
+parallelsDomainGetMaxMemory(virDomainPtr domain)
+{
+parallelsConnPtr privconn = domain->conn->privateData;
+virDomainObjPtr dom = NULL;
+int ret = -1;
+
+dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+if (dom == NULL) {
+parallelsDomNotFoundError(domain);
+return -1;
+}
+
+ret = dom->def->mem.max_balloon;
+virObjectUnlock(dom);
+return ret;
+}
+
 static virHypervisorDriver parallelsDriver = {
 .name = "Parallels",
 .connectOpen = parallelsConnectOpen,/* 0.10.0 */
@@ -1133,6 +1151,7 @@ static virHypervisorDriver parallelsDriver = {
 .domainHasManagedSaveImage = parallelsDomainHasManagedSaveImage, /* 1.2.13 
*/
 .domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */
 .domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 */
+.domainGetMaxMemory = parallelsDomainGetMaxMemory, /* 1.2.14 */
 };
 
 static virConnectDriver parallelsConnectDriver = {
-- 
2.1.0

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

Re: [libvirt] [PATCH] parallels: fix libvirt crash if parallelsNetworkClose fails

[Michal Privoznik]

On 19.03.2015 15:36, Maxim Nestratov wrote:
> If, by any reason, parallelsNetworkClose fails it dereferences
> newly allocated privconn->networks via virObjectUnref, which in
> turn deallocates its memory.
> Subsequent call of parallelsNetworkClose calls virObjectUnref
> that leads to double memory free. To prevent this we should zero
> privconn->networks to make all subsequent virObjectUnref be safe.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/parallels/parallels_network.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/parallels/parallels_network.c 
> b/src/parallels/parallels_network.c
> index 8cc0582..8caad4a 100644
> --- a/src/parallels/parallels_network.c
> +++ b/src/parallels/parallels_network.c
> @@ -348,6 +348,7 @@ parallelsNetworkOpen(virConnectPtr conn,
>  return VIR_DRV_OPEN_SUCCESS;
>   error:
>  virObjectUnref(privconn->networks);
> +privconn->networks = NULL;
>  return VIR_DRV_OPEN_DECLINED;
>  }
>  
> 

This patch is to be ignored since I've pushed the other one:

  https://www.redhat.com/archives/libvir-list/2015-March/msg01002.html

Michal

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

Re: [libvirt] qemu_migration: Precreate missing storage breaks with network drives

[Michal Privoznik]

On 19.03.2015 16:29, Nick Bartos wrote:
> That sounds like a reasonable option.  How far back is the shared disk
> type supported?  As of Icehouse (the oldest supported version I
> believe), the minimum supported libvirt version is 0.9.6.  It would be
> simpler if it could be a simple change that doesn't have to be if/else'd
> for different libvirt versions.
> 

The  element was added back in 0.4.2 release (we're talking
about mid 2008). So I wouldn't fear of it. Here's an example XML taken
from one of our unit tests to show you how to use it:


  
  
  
  
  XYZXYZXYZYXXYZYZYXYZY
  



Michal

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

Re: [libvirt] [PATCH] parallels: fix libvirt crash if parallelsNetworkOpen fails

[Michal Privoznik]

On 19.03.2015 15:43, Maxim Nestratov wrote:
> If, by any reason, parallelsNetworkOpen fails it dereferences
> newly allocated privconn->networks via virObjectUnref, which in
> turn deallocates its memory.
> Subsequent call of parallelsNetworkClose calls virObjectUnref
> that leads to double memory free. To prevent this we should zero
> privconn->networks to make all subsequent virObjectUnref be safe.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/parallels/parallels_network.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/parallels/parallels_network.c 
> b/src/parallels/parallels_network.c
> index 8cc0582..8caad4a 100644
> --- a/src/parallels/parallels_network.c
> +++ b/src/parallels/parallels_network.c
> @@ -348,6 +348,7 @@ parallelsNetworkOpen(virConnectPtr conn,
>  return VIR_DRV_OPEN_SUCCESS;
>   error:
>  virObjectUnref(privconn->networks);
> +privconn->networks = NULL;
>  return VIR_DRV_OPEN_DECLINED;
>  }
>  
> 

ACKed & pushed.

Michal

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

Re: [libvirt] qemu_migration: Precreate missing storage breaks with network drives

[Nick Bartos]

That sounds like a reasonable option.  How far back is the shared disk type
supported?  As of Icehouse (the oldest supported version I believe), the
minimum supported libvirt version is 0.9.6.  It would be simpler if it
could be a simple change that doesn't have to be if/else'd for different
libvirt versions.

On Thu, Mar 19, 2015 at 3:45 AM, Michal Privoznik 
wrote:

> On 18.03.2015 22:00, Nick Bartos wrote:
> > Actually I messed that up slightly on the case.  My C is a bit rusty:
> >
> > diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c
> > libvirt-1.2.13/src/qemu/qemu_migration.c
> > --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c2015-02-23
> > 22:04:12.0 -0800
> > +++ libvirt-1.2.13/src/qemu/qemu_migration.c2015-03-18
> > 13:55:45.873322477 -0700
> > @@ -1507,9 +1507,12 @@
> >  flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
> >  break;
> >
> > +case VIR_STORAGE_TYPE_NETWORK:
> > +ret = 0;
> > +goto cleanup;
> > +break;
> >  case VIR_STORAGE_TYPE_BLOCK:
> >  case VIR_STORAGE_TYPE_DIR:
> > -case VIR_STORAGE_TYPE_NETWORK:
> >  case VIR_STORAGE_TYPE_NONE:
> >  case VIR_STORAGE_TYPE_LAST:
> >  virReportError(VIR_ERR_INTERNAL_ERROR,
>
> Sounds reasonable to me. Although I'm worried that there might be users
> using network disk, supplying different XML on migration, where the disk
> is switched to yet non-existent network disk, expecting libvirt/qemu to
> copy the data.
>
> The other option is: libvirt won't copy shared or read only disks. So
> OpenStack would mark network disks as shared.
>
> One way or another - do you want to propose a patch or should I do that?
>
> Michal
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] parallels: fix libvirt crash if parallelsNetworkOpen fails

[Maxim Nestratov]

If, by any reason, parallelsNetworkOpen fails it dereferences
newly allocated privconn->networks via virObjectUnref, which in
turn deallocates its memory.
Subsequent call of parallelsNetworkClose calls virObjectUnref
that leads to double memory free. To prevent this we should zero
privconn->networks to make all subsequent virObjectUnref be safe.

Signed-off-by: Maxim Nestratov 
---
 src/parallels/parallels_network.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_network.c 
b/src/parallels/parallels_network.c
index 8cc0582..8caad4a 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -348,6 +348,7 @@ parallelsNetworkOpen(virConnectPtr conn,
 return VIR_DRV_OPEN_SUCCESS;
  error:
 virObjectUnref(privconn->networks);
+privconn->networks = NULL;
 return VIR_DRV_OPEN_DECLINED;
 }
 
-- 
1.7.1

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

Re: [libvirt] [PATCH] libxl: use xenlight pkgconfig file if present

[Wei Liu]

On Wed, Mar 18, 2015 at 04:25:41PM -0600, Jim Fehlig wrote:
> xen.git commit babeca32 added a pkgconfig file for libxenlight,
> allowing libxl apps to determine the location of Xen binaries
> such as firmware blobs, device emulator, etc.
> 
> This patch adds support for xenlight.pc in the libxl driver, falling
> back to the previous configure logic if not found.  It introduces
> LIBXL_FIRMWARE_DIR and LIBXL_EXECBIN_DIR to define the firmware and
> libexec_bin locations.  If xenlight.pc does not exist, the defines
> are set to the current hardcoded paths.  The capabilities'
>  and  elements are updated to use the paths.
> 
> Signed-off-by: Jim Fehlig 

Reviewed-by: Wei Liu 

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

[libvirt] [PATCH] parallels: fix libvirt crash if parallelsNetworkClose fails

[Maxim Nestratov]

If, by any reason, parallelsNetworkClose fails it dereferences
newly allocated privconn->networks via virObjectUnref, which in
turn deallocates its memory.
Subsequent call of parallelsNetworkClose calls virObjectUnref
that leads to double memory free. To prevent this we should zero
privconn->networks to make all subsequent virObjectUnref be safe.

Signed-off-by: Maxim Nestratov 
---
 src/parallels/parallels_network.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_network.c 
b/src/parallels/parallels_network.c
index 8cc0582..8caad4a 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -348,6 +348,7 @@ parallelsNetworkOpen(virConnectPtr conn,
 return VIR_DRV_OPEN_SUCCESS;
  error:
 virObjectUnref(privconn->networks);
+privconn->networks = NULL;
 return VIR_DRV_OPEN_DECLINED;
 }
 
-- 
1.7.1

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

Re: [libvirt] [PATCH v4] Automaticly create tap device for VIR_DOMAIN_NET_TYPE_ETHERNET

[Vasiliy Tolstov]

2015-02-27 16:38 GMT+03:00 Vasiliy Tolstov :
> If a user specify ehernet device create it via libvirt and run
> script if it provided. After this commit user does not need to
> run external script to create tap device or add root to qemu
> process.
>
> Signed-off-by: Vasiliy Tolstov 
> ---


ping

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru

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

Re: [libvirt] [PATCH v11] Expose virDomainInterfacesAddresses to python binding

[Pavel Hrdina]

On Wed, Mar 18, 2015 at 10:32:54AM +, Daniel P. Berrange wrote:
> From: Nehal J Wani 
> 
> examples/Makefile.am:
>   * Add new file domipaddrs.py
> 
> examples/README:
>   * Add documentation for the python example
> 
> libvirt-override-api.xml:
>   * Add new symbol for virDomainInterfacesAddresses
> 
> libvirt-override.c:
>   * Hand written python api
> 
> Example:
>   $ python examples/domipaddrs.py qemu:///system f18
> Interface  MAC address  Protocol Address
> vnet0  52:54:00:20:70:3dipv4 192.168.105.240/16
> 
> In v11:
>  - Cope with hwaddr being NULL by filling in PY_NONE
> ---
>  MANIFEST.in  |   1 +
>  examples/README  |   1 +
>  examples/domipaddrs.py   |  57 
>  generator.py |   2 +
>  libvirt-override-api.xml |   9 +++-
>  libvirt-override.c   | 110 
> +++
>  sanitytest.py|   3 ++
>  7 files changed, 182 insertions(+), 1 deletion(-)
>  create mode 100755 examples/domipaddrs.py
> 

[...]

> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
> index 4660c9f..b197639 100644
> --- a/libvirt-override-api.xml
> +++ b/libvirt-override-api.xml
> @@ -678,5 +678,12 @@
>
>
>  
> -  
> +
> +  returns a dictionary of domain interfaces along with their MAC 
> and IP addresses
> +  
> +  
> +  
> +  
> +
> +

Probably just a mistake with the  change.

>  
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 1241305..548be24 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -5120,6 +5120,113 @@ cleanup:
>  return py_retval;
>  }
>  
> +
> +static PyObject *
> +libvirt_virDomainInterfaceAddresses(PyObject *self ATTRIBUTE_UNUSED,
> +PyObject *args)
> +{
> +PyObject *py_retval = VIR_PY_NONE;
> +virDomainPtr domain;
> +PyObject *pyobj_domain;
> +unsigned int source;
> +unsigned int flags;
> +virDomainInterfacePtr *ifaces = NULL;
> +int ifaces_count = 0;
> +size_t i, j;
> +int ret;
> +bool full_free = true;
> +
> +if (!PyArg_ParseTuple(args, (char *) "Oii:virDomainInterfacePtr",
> +  &pyobj_domain, &source, &flags))
> +return NULL;

You cannot return NULL without Py_DECREF(py_retval) because VIR_PY_NONE takes
reference to Py_None.

> +
> +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
> +
> +LIBVIRT_BEGIN_ALLOW_THREADS;
> +ifaces_count = virDomainInterfaceAddresses(domain, &ifaces, source, 
> flags);
> +ret = ifaces_count;
> +LIBVIRT_END_ALLOW_THREADS;
> +if (ret < 0)
> +goto cleanup;

The ret variable is pointless here.

> +
> +if (!(py_retval = PyDict_New()))
> +goto no_memory;
> +
> +for (i = 0; i < ifaces_count; i++) {
> +virDomainInterfacePtr iface = ifaces[i];
> +PyObject *py_addrs = NULL;
> +PyObject *py_iface = NULL;
> +
> +if (!(py_iface = PyDict_New()))
> +goto no_memory;
> +
> +if (iface->naddrs) {
> +if (!(py_addrs = PyList_New(iface->naddrs))) {
> +Py_DECREF(py_iface);
> +goto no_memory;
> +}
> +} else {
> +py_addrs = VIR_PY_NONE;
> +}
> +
> +for (j = 0; j < iface->naddrs; j++) {
> +virDomainIPAddressPtr addr = &(iface->addrs[j]);
> +PyObject *py_addr = PyDict_New();
> +int type = addr->type;
> +
> +if (!addr) {

You've probably meant py_addr.

> +Py_DECREF(py_iface);
> +Py_DECREF(py_addrs);
> +goto no_memory;
> +}
> +
> +PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("addr"),
> +  libvirt_constcharPtrWrap(addr->addr));
> +PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("prefix"),
> +   libvirt_intWrap(addr->prefix));
> +PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("type"),
> +   libvirt_intWrap(type));
> +

All the libvirt_*Wrap and Py*_SetItem functions can fail.  In that case they
returns NULL/-1 and set an exception and we should check that and in case of
error return NULL.

> +PyList_SetItem(py_addrs, j, py_addr);
> +}
> +
> +PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("addrs"),
> +   py_addrs);
> +if (iface->hwaddr) {
> +PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("hwaddr"),
> +   libvirt_constcharPtrWrap(iface->hwaddr));
> +} else {
> +PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("hwaddr"),
> +   VIR_PY_NONE);
> +}
> +
> +PyDict_SetItem(py_retval, libvirt_charPtrWrap(iface->name),
> +   py_iface);
> +}
> +
> +full_free = false;
> +
> +

[libvirt] [libvirt-python PATCH v12] Expose virDomainInterfacesAddresses to python binding

[Pavel Hrdina]

From: Nehal J Wani 

examples/Makefile.am:
  * Add new file domipaddrs.py

examples/README:
  * Add documentation for the python example

libvirt-override-api.xml:
  * Add new symbol for virDomainInterfacesAddresses

libvirt-override.c:
  * Hand written python api

Example:
  $ python examples/domipaddrs.py qemu:///system f18
Interface  MAC address  Protocol Address
vnet0  52:54:00:20:70:3dipv4 192.168.105.240/16

In v11:
 - Cope with hwaddr being NULL by filling in PY_NONE

Signed-off-by: Pavel Hrdina 
---
 MANIFEST.in  |   1 +
 examples/README  |   1 +
 examples/domipaddrs.py   |  57 +
 generator.py |   2 +
 libvirt-override-api.xml |   7 +++
 libvirt-override.c   | 128 +++
 sanitytest.py|   3 ++
 7 files changed, 199 insertions(+)
 create mode 100755 examples/domipaddrs.py

diff --git a/MANIFEST.in b/MANIFEST.in
index d7bc545..dd05221 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -4,6 +4,7 @@ include COPYING
 include COPYING.LESSER
 include ChangeLog
 include examples/consolecallback.py
+include examples/domipaddrs.py
 include examples/dominfo.py
 include examples/domrestore.py
 include examples/domsave.py
diff --git a/examples/README b/examples/README
index 5b5d405..1d4b425 100644
--- a/examples/README
+++ b/examples/README
@@ -11,6 +11,7 @@ domrestore.py - restore domU's from their saved files in a 
directory
 esxlist.py  - list active domains of an VMware ESX host and print some info.
   also demonstrates how to use the libvirt.openAuth() method
 dhcpleases.py - list dhcp leases for a given virtual network
+domipaddrs.py - list IP addresses for guest domains
 
 The XML files in this directory are examples of the XML format that libvirt
 expects, and will have to be adapted for your setup. They are only needed
diff --git a/examples/domipaddrs.py b/examples/domipaddrs.py
new file mode 100755
index 000..d6d5cac
--- /dev/null
+++ b/examples/domipaddrs.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+# domipaddrs - print domain interfaces along with their MAC and IP addresses
+
+import libvirt
+import sys
+
+def usage():
+print "Usage: %s [URI] DOMAIN" % sys.argv[0]
+print "Print domain interfaces along with their MAC and IP 
addresses"
+
+uri = None
+name = None
+args = len(sys.argv)
+
+if args == 2:
+name = sys.argv[1]
+elif args == 3:
+uri = sys.argv[1]
+name = sys.argv[2]
+else:
+usage()
+sys.exit(2)
+
+conn = libvirt.open(uri)
+if conn == None:
+print "Unable to open connection to libvirt"
+sys.exit(1)
+
+try:
+dom = conn.lookupByName(name)
+except libvirt.libvirtError:
+print "Domain %s not found" % name
+sys.exit(0)
+
+ifaces = 
dom.interfaceAddresses(libvirt.VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE);
+if (ifaces == None):
+print "Failed to get domain interfaces"
+sys.exit(0)
+
+print " {0:10} {1:20} {2:12} {3}".format("Interface", "MAC address", 
"Protocol", "Address")
+
+def toIPAddrType(addrType):
+if addrType == libvirt.VIR_IP_ADDR_TYPE_IPV4:
+return "ipv4"
+elif addrType == libvirt.VIR_IP_ADDR_TYPE_IPV6:
+return "ipv6"
+
+for (name, val) in ifaces.iteritems():
+if val['addrs']:
+for addr in val['addrs']:
+   print " {0:10} {1:19}".format(name, val['hwaddr']),
+   print " {0:12} {1}/{2} ".format(toIPAddrType(addr['type']), 
addr['addr'], addr['prefix']),
+   print
+else:
+print " {0:10} {1:19}".format(name, val['hwaddr']),
+print " {0:12} {1}".format("N/A", "N/A"),
+print
diff --git a/generator.py b/generator.py
index df7a74d..8c1c48e 100755
--- a/generator.py
+++ b/generator.py
@@ -483,6 +483,7 @@ skip_impl = (
 'virDomainBlockCopy',
 'virNodeAllocPages',
 'virDomainGetFSInfo',
+'virDomainInterfaceAddresses',
 )
 
 lxc_skip_impl = (
@@ -595,6 +596,7 @@ skip_function = (
 'virDomainStatsRecordListFree', # only useful in C, python uses dict
 'virDomainFSInfoFree', # only useful in C, python code uses list
 'virDomainIOThreadsInfoFree', # only useful in C, python code uses list
+'virDomainInterfaceFree', # only useful in C, python code uses list
 )
 
 lxc_skip_function = (
diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
index 4660c9f..40ad602 100644
--- a/libvirt-override-api.xml
+++ b/libvirt-override-api.xml
@@ -678,5 +678,12 @@
   
   
 
+
+  returns a dictionary of domain interfaces along with their MAC and 
IP addresses
+  
+  
+  
+  
+
   
 
diff --git a/libvirt-override.c b/libvirt-override.c
index 1241305..b64c101 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -5120,6 +5120,131 @@ cleanup:
 return py_retval;
 }
 
+
+#if LIBVIR_CHECK_VERSION(1, 2, 14)
+static PyObject *
+libvirt_virDomainInterfaceAddresses(PyObject *self ATTRIBUTE_UNUSED,
+PyOb

[libvirt] LXC container with user namespace and root fs on loop device - how it's supposed to work?

[Dmitry Guryanov]

Hello,

It's not possible to start LXC container inside user namespace with root 
filesystem on loop device, because it tries to mount root FS from 
container's user namespace (lxcContainerSetupPivotRoot) and gets EPERM:


2015-03-19 12:48:18.545+: 1: debug : lxcContainerChild:2278 : 
Tearing down container
Failed to mount device /dev/loop0 to 
/var/run/libvirt/lxc/instance-000b.root: Operation not permitted




So I wonder, if someone tried to run LXC container with such 
configuration with success.



Here is my config:


  instance-000b
  d918c415-0a00-4c12-896e-19e471d3
  524288
  524288
  
1024
  
  
exe
/sbin/init
console=tty0 console=ttyS0
  
  


  
  
  destroy
  restart
  destroy
  
/usr/libexec/libvirt_lxc

  
  
  


  

  


--
Dmitry Guryanov

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

[libvirt] [PATCH v2 4/6] parallels: report, that cdroms are readonly

[Dmitry Guryanov]

When retriving list of domains from PCS, set
readonly flag for cdrom devices.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_sdk.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 7a90eec..a49f792 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -478,10 +478,12 @@ prlsdkGetDiskInfo(PRL_HANDLE prldisk,
 virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
 }
 
-if (isCdrom)
+if (isCdrom) {
 disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
-else
+disk->src->readonly = true;
+} else {
 disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
+}
 
 pret = PrlVmDev_GetFriendlyName(prldisk, NULL, &buflen);
 prlsdkCheckRetGoto(pret, cleanup);
-- 
2.1.0

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

[libvirt] [PATCH v2 5/6] parallels: add controllers in prlsdkLoadDomain

[Dmitry Guryanov]

Call virDomainDefAddImplicitControllers to add disk
controllers, so virDomainDef, filled by this function
will look exactly like the one returned by virDomainDefParseString.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_sdk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index a49f792..888a3eb 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1265,6 +1265,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
 *s = '\0';
 }
 
+if (virDomainDefAddImplicitControllers(def) < 0)
+goto error;
+
 if (olddom) {
 /* assign new virDomainDef without any checks */
 /* we can't use virDomainObjAssignDef, because it checks
-- 
2.1.0

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

[libvirt] [PATCH v2 6/6] parallels: fix virDomainDefineXML for domain in saved state

[Dmitry Guryanov]

PCS doesn't store domain config in managed save state file.
It's forbidden to change config for VMs in this state.
It's possible to change config for containers, but after
restoring domain will have that new config, not a config,
which domain had at the moment of virDomainManagedSave.

So we need to handle this case differently from other states.
Let's forbid this operation, if config is changed and if it's
not changed - just do nothing.

Openstack/nova calls virDomainDefineXML on resume with
current domain config, so we can't forbid this operation
in managed save state.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_driver.c | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 9bbb970..391e927 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -723,11 +723,35 @@ parallelsDomainDefineXMLFlags(virConnectPtr conn, const 
char *xml, unsigned int
 if (!olddom)
 goto cleanup;
 } else {
-if (prlsdkApplyConfig(conn, olddom, def))
-goto cleanup;
+int state, reason;
+
+state = virDomainObjGetState(olddom, &reason);
+
+if (state == VIR_DOMAIN_SHUTOFF &&
+reason == VIR_DOMAIN_SHUTOFF_SAVED) {
+
+/* PCS doesn't store domain config in managed save state file.
+ * It's forbidden to change config for VMs in this state.
+ * It's possible to change config for containers, but after
+ * restoring domain will have that new config, not a config,
+ * which domain had at the moment of virDomainManagedSave.
+ *
+ * So forbid this operation, if config is changed. If it's
+ * not changed - just do nothing. */
+
+if (!virDomainDefCheckABIStability(olddom->def, def)) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("Can't change domain configuration "
+ "in managed save state"));
+goto cleanup;
+}
+} else {
+if (prlsdkApplyConfig(conn, olddom, def))
+goto cleanup;
 
-if (prlsdkUpdateDomain(privconn, olddom))
-goto cleanup;
+if (prlsdkUpdateDomain(privconn, olddom))
+goto cleanup;
+}
 }
 
 retdom = virGetDomain(conn, def->name, def->uuid);
-- 
2.1.0

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

[libvirt] [PATCH v2 0/6] parallels: implement managed save

[Dmitry Guryanov]

This patch series is intended to implement all needed code, so
that suspend/resume in openstack nova will work.

It implements .domainHasManagedSaveImage, .domainManagedSave
and .domainManagedSaveRemove functions. Also it adds
workaround to parallelsDomainDefineXMLFlags to skip
applying configuration, if VM is in managed save state and
config hasn't been changed, because it's not possible
to change VM config in suspended state in PCS.

Changes in v2:
* rebased

Dmitry Guryanov (6):
  parallels: fix headers in parallels_sdk.h
  parallels: split prlsdkDomainChangeState function
  parallels: implement virDomainManagedSave
  parallels: report, that cdroms are readonly
  parallels: add controllers in prlsdkLoadDomain
  parallels: fix virDomainDefineXML for domain in saved state

 src/parallels/parallels_driver.c | 101 +--
 src/parallels/parallels_sdk.c|  65 ++---
 src/parallels/parallels_sdk.h|  17 +--
 3 files changed, 157 insertions(+), 26 deletions(-)

-- 
2.1.0

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

[libvirt] [PATCH v2 1/6] parallels: fix headers in parallels_sdk.h

[Dmitry Guryanov]

Return value of functions prlsdkStart/Kill/Stop e.t.c.
is PRL_RESULT in parallels_sdk.c and int in parallels_sdk.h.
PRL_RESULT is int, so compiler didn't report errors.
Let's fix the difference.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_sdk.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h
index 694c19b..cb8d3fb 100644
--- a/src/parallels/parallels_sdk.h
+++ b/src/parallels/parallels_sdk.h
@@ -35,11 +35,11 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned 
char *uuid);
 int prlsdkUpdateDomain(parallelsConnPtr privconn, virDomainObjPtr dom);
 int prlsdkSubscribeToPCSEvents(parallelsConnPtr privconn);
 void prlsdkUnsubscribeFromPCSEvents(parallelsConnPtr privconn);
-int prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
-int prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
-int prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
-int prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
-int prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
+PRL_RESULT prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
+PRL_RESULT prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
+PRL_RESULT prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
+PRL_RESULT prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
+PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
 
 typedef PRL_RESULT (*prlsdkChangeStateFunc)(parallelsConnPtr privconn, 
PRL_HANDLE sdkdom);
 int
-- 
2.1.0

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

[libvirt] [PATCH v2 3/6] parallels: implement virDomainManagedSave

[Dmitry Guryanov]

Implement virDomainManagedSave api function. In PCS
this feature called "suspend". You can suspend VM or
CT while it is in running or paused state. And after
resuming (or starting) it will have the same state, as
before suspend.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_driver.c | 69 +++-
 src/parallels/parallels_sdk.c| 21 
 src/parallels/parallels_sdk.h|  3 ++
 3 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 2a6a7c9..9bbb970 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -986,6 +986,8 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, 
unsigned int flags)
 {
 parallelsConnPtr privconn = domain->conn->privateData;
 virDomainObjPtr dom = NULL;
+int state, reason;
+int ret = 0;
 
 virCheckFlags(0, -1);
 
@@ -995,9 +997,72 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, 
unsigned int flags)
 return -1;
 }
 
+state = virDomainObjGetState(dom, &reason);
+if (state == VIR_DOMAIN_SHUTOFF && reason == VIR_DOMAIN_SHUTOFF_SAVED)
+ret = 1;
 virObjectUnlock(dom);
 
-return 0;
+return ret;
+}
+
+static int
+parallelsDomainManagedSave(virDomainPtr domain, unsigned int flags)
+{
+parallelsConnPtr privconn = domain->conn->privateData;
+virDomainObjPtr dom = NULL;
+int state, reason;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_SAVE_RUNNING |
+  VIR_DOMAIN_SAVE_PAUSED, -1);
+
+dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+if (dom == NULL) {
+parallelsDomNotFoundError(domain);
+return -1;
+}
+
+state = virDomainObjGetState(dom, &reason);
+
+if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) {
+ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkPause);
+if (ret)
+goto cleanup;
+}
+
+ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkSuspend);
+
+ cleanup:
+virObjectUnlock(dom);
+return ret;
+}
+
+static int
+parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags)
+{
+parallelsConnPtr privconn = domain->conn->privateData;
+virDomainObjPtr dom = NULL;
+int state, reason;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+if (dom == NULL) {
+parallelsDomNotFoundError(domain);
+return -1;
+}
+
+state = virDomainObjGetState(dom, &reason);
+
+if (!(state == VIR_DOMAIN_SHUTOFF && reason == VIR_DOMAIN_SHUTOFF_SAVED))
+goto cleanup;
+
+ret = prlsdkDomainManagedSaveRemove(privconn, dom);
+
+ cleanup:
+virObjectUnlock(dom);
+return ret;
 }
 
 static virHypervisorDriver parallelsDriver = {
@@ -1042,6 +1107,8 @@ static virHypervisorDriver parallelsDriver = {
 .connectIsSecure = parallelsConnectIsSecure, /* 1.2.5 */
 .connectIsAlive = parallelsConnectIsAlive, /* 1.2.5 */
 .domainHasManagedSaveImage = parallelsDomainHasManagedSaveImage, /* 1.2.13 
*/
+.domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */
+.domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 */
 };
 
 static virConnectDriver parallelsConnectDriver = {
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index d985a93..7a90eec 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1720,6 +1720,14 @@ PRL_RESULT prlsdkResume(parallelsConnPtr privconn, 
PRL_HANDLE sdkdom)
 return waitJob(job, privconn->jobTimeout);
 }
 
+PRL_RESULT prlsdkSuspend(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
+{
+PRL_HANDLE job = PRL_INVALID_HANDLE;
+
+job = PrlVm_Suspend(sdkdom);
+return waitJob(job, privconn->jobTimeout);
+}
+
 int
 prlsdkDomainChangeStateLocked(parallelsConnPtr privconn,
   virDomainObjPtr dom,
@@ -3204,3 +3212,16 @@ prlsdkUnregisterDomain(parallelsConnPtr privconn, 
virDomainObjPtr dom)
 virDomainObjListRemove(privconn->domains, dom);
 return 0;
 }
+
+int
+prlsdkDomainManagedSaveRemove(parallelsConnPtr privconn, virDomainObjPtr dom)
+{
+parallelsDomObjPtr privdom = dom->privateData;
+PRL_HANDLE job;
+
+job = PrlVm_DropSuspendedState(privdom->sdkdom);
+if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
+return -1;
+
+return 0;
+}
diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h
index 780a226..b084678 100644
--- a/src/parallels/parallels_sdk.h
+++ b/src/parallels/parallels_sdk.h
@@ -40,6 +40,7 @@ PRL_RESULT prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE 
sdkdom);
 PRL_RESULT prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
 PRL_RESULT prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
 PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
+PRL_RESULT

[libvirt] [PATCH v2 2/6] parallels: split prlsdkDomainChangeState function

[Dmitry Guryanov]

Split function prlsdkDomainChangeState into
prlsdkDomainChangeStateLocked and prlsdkDomainChangeState.
So it can be used from places, where virDomainObj already
found and locked.

Signed-off-by: Dmitry Guryanov 
---
 src/parallels/parallels_sdk.c | 35 +--
 src/parallels/parallels_sdk.h |  4 
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index c36b772..d985a93 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1721,22 +1721,14 @@ PRL_RESULT prlsdkResume(parallelsConnPtr privconn, 
PRL_HANDLE sdkdom)
 }
 
 int
-prlsdkDomainChangeState(virDomainPtr domain,
-prlsdkChangeStateFunc chstate)
+prlsdkDomainChangeStateLocked(parallelsConnPtr privconn,
+  virDomainObjPtr dom,
+  prlsdkChangeStateFunc chstate)
 {
-parallelsConnPtr privconn = domain->conn->privateData;
-virDomainObjPtr dom;
 parallelsDomObjPtr pdom;
 PRL_RESULT pret;
-int ret = -1;
 virErrorNumber virerr;
 
-dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
-if (dom == NULL) {
-parallelsDomNotFoundError(domain);
-return -1;
-}
-
 pdom = dom->privateData;
 pret = chstate(privconn, pdom->sdkdom);
 if (PRL_FAILED(pret)) {
@@ -1752,12 +1744,27 @@ prlsdkDomainChangeState(virDomainPtr domain,
 }
 
 virReportError(virerr, "%s", _("Can't change domain state."));
-goto cleanup;
+return -1;
 }
 
-ret = prlsdkUpdateDomain(privconn, dom);
+return prlsdkUpdateDomain(privconn, dom);
+}
 
- cleanup:
+int
+prlsdkDomainChangeState(virDomainPtr domain,
+prlsdkChangeStateFunc chstate)
+{
+parallelsConnPtr privconn = domain->conn->privateData;
+virDomainObjPtr dom;
+int ret = -1;
+
+dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+if (dom == NULL) {
+parallelsDomNotFoundError(domain);
+return -1;
+}
+
+ret = prlsdkDomainChangeStateLocked(privconn, dom, chstate);
 virObjectUnlock(dom);
 return ret;
 }
diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h
index cb8d3fb..780a226 100644
--- a/src/parallels/parallels_sdk.h
+++ b/src/parallels/parallels_sdk.h
@@ -46,6 +46,10 @@ int
 prlsdkDomainChangeState(virDomainPtr domain,
 prlsdkChangeStateFunc chstate);
 int
+prlsdkDomainChangeStateLocked(parallelsConnPtr privconn,
+  virDomainObjPtr dom,
+  prlsdkChangeStateFunc chstate);
+int
 prlsdkApplyConfig(virConnectPtr conn,
   virDomainObjPtr dom,
   virDomainDefPtr new);
-- 
2.1.0

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

Re: [libvirt] USB hostdev de/reattaching

[Ján Tomko]

On Thu, Mar 19, 2015 at 06:12:44AM -0400, Martin Polednik wrote:
> Hello,
> 
> according to libvirt documentation, when using hostdev with USB device
> "...the user is responsible to call virNodeDeviceDettach (or virsh 
> nodedev-detach)
> before starting the guest...". In libvirt-1.2.13-1.el7.x86_64, this leads to 
> 
> virsh nodedev-detach usb_usb1
> 
> error: Failed to detach device usb_usb1
> error: invalid argument: device usb_usb1 is not a PCI device
> 
> The failure is most probably caused in qemuNodeDeviceDeta where 
> qemuNodeDeviceGetPCIInfo
> is called. Is it a bug, or usb hostdev can be done without 
> detaching/reattaching?

USB devices do not need to be detached/reattached explicitly, it's all
automatic.

And since qemuNodeDeviceDettach has always required a PCI device (see
commit 34d23b0), I think this is a documentation bug, introduced by
commit 10d3272.

Jan


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

Re: [libvirt] qemu_migration: Precreate missing storage breaks with network drives

[Michal Privoznik]

On 18.03.2015 22:00, Nick Bartos wrote:
> Actually I messed that up slightly on the case.  My C is a bit rusty:
> 
> diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c
> libvirt-1.2.13/src/qemu/qemu_migration.c
> --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c2015-02-23
> 22:04:12.0 -0800
> +++ libvirt-1.2.13/src/qemu/qemu_migration.c2015-03-18
> 13:55:45.873322477 -0700
> @@ -1507,9 +1507,12 @@
>  flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
>  break;
>  
> +case VIR_STORAGE_TYPE_NETWORK:
> +ret = 0;
> +goto cleanup;
> +break;
>  case VIR_STORAGE_TYPE_BLOCK:
>  case VIR_STORAGE_TYPE_DIR:
> -case VIR_STORAGE_TYPE_NETWORK:
>  case VIR_STORAGE_TYPE_NONE:
>  case VIR_STORAGE_TYPE_LAST:
>  virReportError(VIR_ERR_INTERNAL_ERROR,

Sounds reasonable to me. Although I'm worried that there might be users
using network disk, supplying different XML on migration, where the disk
is switched to yet non-existent network disk, expecting libvirt/qemu to
copy the data.

The other option is: libvirt won't copy shared or read only disks. So
OpenStack would mark network disks as shared.

One way or another - do you want to propose a patch or should I do that?

Michal

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

Re: [libvirt] [SOLVED] Domain XML isn't dumping full backing chain

[Deepak Shetty]

On Wed, Mar 18, 2015 at 5:33 PM, Shanzhi Yu  wrote:

>
>
> --
>
> *From: *"Deepak Shetty" 
> *To: *libvir-list@redhat.com
> *Sent: *Wednesday, March 18, 2015 7:19:05 PM
> *Subject: *[libvirt] Domain XML isn't dumping full backing chain
>
>
> Hi,
> I am using libvirt version 1.2.9.2 on F21 and i am unable to get the
> complete backing chain info in the virsh dumpxml output. Details below :
>
> *My backing chain per qemu-img :*
>
> [stack@devstack-f21 test]$ qemu-img info --backing-chain snap4.qcow2
> image: snap4.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 196K
> cluster_size: 65536
> backing file: ./snap3.qcow2
> Format specific information:
> compat: 1.1
> lazy refcounts: false
>
> image: ./snap3.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 196K
> cluster_size: 65536
> backing file: ./snap2.qcow2 (actual path: ././snap2.qcow2)
> Format specific information:
> compat: 1.1
> lazy refcounts: false
>
> image: ././snap2.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 196K
> cluster_size: 65536
> backing file: ./snap1.qcow2 (actual path: ./././snap1.qcow2)
> Format specific information:
> compat: 1.1
> lazy refcounts: false
>
> image: ./././snap1.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 196K
> cluster_size: 65536
> backing file: ./base.qcow2 (actual path: ././././base.qcow2)
> Format specific information:
> compat: 1.1
> lazy refcounts: false
>
> image: ././././base.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 196K
> cluster_size: 65536
> Format specific information:
> compat: 1.1
> lazy refcounts: false
>
> If you want prepare the backing chain yourself, you should  add "-o
> backing_fmt=$farmat" options,
> like "qemu-img create -f qcow2 base.s1 -b base.qcow2 -o backing_fmt=qcow2"
>

Thanks, it works after I did the above

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

[libvirt] USB hostdev de/reattaching

[Martin Polednik]

Hello,

according to libvirt documentation, when using hostdev with USB device
"...the user is responsible to call virNodeDeviceDettach (or virsh 
nodedev-detach)
before starting the guest...". In libvirt-1.2.13-1.el7.x86_64, this leads to 

virsh nodedev-detach usb_usb1

error: Failed to detach device usb_usb1
error: invalid argument: device usb_usb1 is not a PCI device

The failure is most probably caused in qemuNodeDeviceDeta where 
qemuNodeDeviceGetPCIInfo
is called. Is it a bug, or usb hostdev can be done without 
detaching/reattaching?

Thanks,
mpolednik 

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

[libvirt] [PATCH] conf: fix running vm numa settings disappear after restart libvirtd

[Luyao Huang]

5bba61f introduce a issue : when start a vm with  settings and
restart libvirtd, numa settings will disappear. Because when parse the
vm states file, there is no node in "/domain/cpu/numa" this place.

Change to use ./cpu/numa instead of /domain/cpu/numa.

Signed-off-by: Luyao Huang 
---
 src/conf/numa_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index b92cb44..c34ba5f 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -663,10 +663,10 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def,
 int ret = -1;
 
 /* check if NUMA definition is present */
-if (!virXPathNode("/domain/cpu/numa[1]", ctxt))
+if (!virXPathNode("./cpu/numa[1]", ctxt))
 return 0;
 
-if ((n = virXPathNodeSet("/domain/cpu/numa[1]/cell", ctxt, &nodes)) <= 0) {
+if ((n = virXPathNodeSet("./cpu/numa[1]/cell", ctxt, &nodes)) <= 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("NUMA topology defined without NUMA cells"));
 goto cleanup;
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] qemu: fix sometimes error overwrite when we fail to start/migrate/restore

[lhuang]

On 03/19/2015 05:27 PM, Ján Tomko wrote:

On Thu, Mar 19, 2015 at 11:14:39AM +0800, Luyao Huang wrote:

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


Adding a comment in the bugzilla saying you posted a patch (with a link
to libvir-list archives) can be helpful. Otherwise they might send the
exact same patch - if your was not merged yet, or it was rejected.


got it, I will add a comment next time and thank a lot for your help to 
add them before.



When start/migrate/restore a vm failed, libvirt will try to catch the log in

Libvirt only uses the error from qemu if qemu exits during the startup.
Start/migrate/restore could fail for other reasons.


/var/log/libvirt/qemu/vm.log and output them before. However we add a check in
qemuDomainObjExitMonitor after commit dc2fd51f, this will overwrite
the error set in priv->mon which has set by qemuMonitorIO (a qemu monitor
I/O event callback function).

qemuMonitorIO only stores the error in mon->lastError.
It's the functions like qemuMonitorSend and qemuMonitorClose that take
this error and set it via virSetError.


Oh, I see.


Add a check in qemuDomainObjExitMonitor, if there is an error have been
set by other function, we won't overwrite it.

Reworded as:
 When qemu exits during startup, libvirt includes the error from
 /var/log/libvirt/qemu/vm.log in the error message:
 
 $ virsh start test3

 error: Failed to start domain test3
 error: internal error: early end of file from monitor: possible problem:
 2015-02-27T03:03:16.985494Z qemu-kvm: -numa memdev is not supported by
 machine rhel6.5.0
 
 The check for domain liveness added to qemuDomainObjExitMonitor

 in commit dc2fd51f sometimes overwrites this error:
 $ virsh start test3
 error: Failed to start domain test3
 error: operation failed: domain is no longer running
 
 Fix the check to only report an error if there is none set.


Thanks a lot for the reword.


Signed-off-by: Luyao Huang 
---
  src/qemu/qemu_domain.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)


ACK and pushed.

Jan


Thanks for your review.


diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2eacef2..41d1263 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1609,8 +1609,9 @@ int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
  {
  qemuDomainObjExitMonitorInternal(driver, obj);
  if (!virDomainObjIsActive(obj)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("domain is no longer running"));
+if (!virGetLastError())
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("domain is no longer running"));
  return -1;
  }
  return 0;
--
1.8.3.1

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


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

Re: [libvirt] [PATCH] qemu: skip precreation of network disks

[Michael Chapman]

On Fri, 13 Mar 2015, Michael Chapman wrote:

Commit cf54c60699833b3791a5d0eb3eb5a1948c267f6b introduced the ability
to create missing storage volumes during migration. For network disks,
however, we may not necessarily be able to detect whether they already
exist -- there is no straight-forward way to map the disk to a storage
volume, and even if there were it's possible no configured storage pool
actually contains the disk.

It is better to assume the network disk exists in this case, rather than
aborting the migration completely. If the volume really is missing, QEMU
will generate an appropriate error later in the migration.

Signed-off-by: Michael Chapman 
---
src/qemu/qemu_migration.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)


Ping?

Looks like Noel Burton-Krahn / Nick Bartos brought up the same issue on 
the list today (https://www.redhat.com/archives/libvir-list/2015-March/msg00969.html). 
I'd be happy for either patch to go in.


- Michael

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

Re: [libvirt] [PATCH] qemu: fix sometimes error overwrite when we fail to start/migrate/restore

[Ján Tomko]

On Thu, Mar 19, 2015 at 11:14:39AM +0800, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1196934
> 

Adding a comment in the bugzilla saying you posted a patch (with a link
to libvir-list archives) can be helpful. Otherwise they might send the
exact same patch - if your was not merged yet, or it was rejected.

> When start/migrate/restore a vm failed, libvirt will try to catch the log in

Libvirt only uses the error from qemu if qemu exits during the startup.
Start/migrate/restore could fail for other reasons.

> /var/log/libvirt/qemu/vm.log and output them before. However we add a check in
> qemuDomainObjExitMonitor after commit dc2fd51f, this will overwrite
> the error set in priv->mon which has set by qemuMonitorIO (a qemu monitor
> I/O event callback function).

qemuMonitorIO only stores the error in mon->lastError.
It's the functions like qemuMonitorSend and qemuMonitorClose that take
this error and set it via virSetError.

> 
> Add a check in qemuDomainObjExitMonitor, if there is an error have been
> set by other function, we won't overwrite it.

Reworded as:
When qemu exits during startup, libvirt includes the error from
/var/log/libvirt/qemu/vm.log in the error message:

$ virsh start test3
error: Failed to start domain test3
error: internal error: early end of file from monitor: possible problem:
2015-02-27T03:03:16.985494Z qemu-kvm: -numa memdev is not supported by
machine rhel6.5.0

The check for domain liveness added to qemuDomainObjExitMonitor
in commit dc2fd51f sometimes overwrites this error:
$ virsh start test3
error: Failed to start domain test3
error: operation failed: domain is no longer running

Fix the check to only report an error if there is none set.

> 
> Signed-off-by: Luyao Huang 
> ---
>  src/qemu/qemu_domain.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

ACK and pushed.

Jan

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2eacef2..41d1263 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1609,8 +1609,9 @@ int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
>  {
>  qemuDomainObjExitMonitorInternal(driver, obj);
>  if (!virDomainObjIsActive(obj)) {
> -virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -   _("domain is no longer running"));
> +if (!virGetLastError())
> +virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +   _("domain is no longer running"));
>  return -1;
>  }
>  return 0;
> -- 
> 1.8.3.1
> 
> --
> 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