Re: [libvirt] Enhancing block/disk migration in libvirt

[Tony Breeds]

On Mon, Feb 16, 2015 at 11:34:51AM +0100, Michal Privoznik wrote:
> On 16.02.2015 11:16, Daniel P. Berrange wrote:
> > 
> > This doesn't really belong in the XML. The XML configs are for controlling
> > the guest configuration, not functional behaviour of the APIs. So we need
> > to fit any neccessary information into the migration API parameters instead,
> > or define a new migration API for it. At minimum we just want a list of
> > disks to be migrated.
> 
> We have a migrate API that takes an array of virTypedParameter. We can
> use that to let users pass a list of disks to migrate.

Thanks I'll look at adding that to the API.
 
Yours Tony.


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

Re: [libvirt] [PATCH] Add support for the midonet virtualport type

[Laine Stump]

On 02/16/2015 08:46 PM, Antoni Segura Puimedon wrote:
> From: Antoni Segura Puimedon 
>
> Midonet is an opensource virtual networking that over lays the IP
> network between hypervisors. Currently, such networks can be made
> with the openvswitch virtualport type. With this patch, one will
> be able to setup such configurations by binding vNICs to Midonet
> overlay ports.
>
> Signed-off-by: Antoni Segura Puimedon 
> ---
>  configure.ac |  4 ++
>  docs/schemas/networkcommon.rng   | 12 +
>  src/Makefile.am  |  1 +
>  src/conf/domain_conf.h   |  1 +
>  src/conf/netdev_vport_profile_conf.c |  3 +-
>  src/libvirt_private.syms |  5 ++
>  src/qemu/qemu_hotplug.c  | 25 +++---
>  src/qemu/qemu_process.c  | 13 +++--
>  src/util/virnetdevmidonet.c  | 97 
> 
>  src/util/virnetdevmidonet.h  | 37 ++
>  src/util/virnetdevtap.c  | 11 ++--
>  src/util/virnetdevvportprofile.c |  1 +
>  src/util/virnetdevvportprofile.h |  5 +-
>  13 files changed, 197 insertions(+), 18 deletions(-)
>  create mode 100644 src/util/virnetdevmidonet.c
>  create mode 100644 src/util/virnetdevmidonet.h

I haven't checked the rest for completeness yet, but thought I'd point
out in advance that you need to also document the new type in
formatdomain.html.in.

Also, I don't see any changes here to the network configuration to
support a network of this type. That can be a second step though. (A
very useful thing - it would allow migration of a guest from a host
using a standard Linux host bridge to a host using this new network
connection type, for example).

You may also need to go through the hypervisor drivers other than qemu
and put in explicit checks for your new virtualport type and log an
UNSUPPORTED message if it's encountered.

Another thing - for awhile now everyone has been putting the
config/doc/schema changes in one patch, the utility functions in a
second, and the glue that ties them together in qemu/lxc/etc in a third.
That can make it easier to review, test, and also in the case that some
other new feature might use the same utility functions it makes it
easier to backport one feature without needing to backport the other.
etc etc.


I'll go through the code that that's here in the morning.


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

Re: [libvirt] [PATCH] virsh: fix IP address in vncdisplay for listen type='network'

[lhuang]

On 02/16/2015 06:51 PM, Pavel Hrdina wrote:

On Sun, Feb 15, 2015 at 04:49:09PM +0800, Luyao Huang wrote:

Just like the fix for domdisplay in commit 1ba815.
---
  tools/virsh-domain.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index dc4a863..2506b89 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10269,6 +10269,18 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd)
  
  listen_addr = virXPathString("string(/domain/devices/graphics"

   "[@type='vnc']/@listen)", ctxt);
+if (!listen_addr) {
+/* The subelement address -  -
+ * *should* have been automatically backfilled into its
+ * parent  (which we just tried to
+ * retrieve into listen_addr above) but in some cases it
+ * isn't, so we also do an explicit check for the
+ * subelement (which, by the way, doesn't exist on libvirt
+ * < 0.9.4, so we really do need to check both places)
+ */
+listen_addr = virXPathString("string(/domain/devices/graphics"
+ "[@type='vnc']/listen/@address)", ctxt);
+}
  if (listen_addr == NULL || STREQ(listen_addr, "0.0.0.0"))
  vshPrint(ctl, ":%d\n", port-5900);
  else
--
1.8.3.1


ACK and pushed.


Thanks for the quick review.

Pavel


--
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

[libvirt] [PATCH] qemu: fix vm deadlock when try to use numatune in session mode

[Luyao Huang]

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

commit 43b67f introduce a deadlock issue when we use numatune
to change numa settings to a vm in session mode.

Jump to endjob instead of jump to cleanup.

Signed-off-by: Luyao Huang 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 709f468..1bbbe9b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9495,7 +9495,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 flags & VIR_DOMAIN_AFFECT_LIVE) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("NUMA tuning is not available in session mode"));
-goto cleanup;
+goto endjob;
 }
 
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-- 
1.8.3.1

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

Re: [libvirt] Enhancing block/disk migration in libvirt

[Tony Breeds]

On Mon, Feb 16, 2015 at 11:12:14AM +0100, Kashyap Chamarthy wrote:

> On a related note, some weeks ago, I captured some context and analysis
> (including some debugging done by Dan Berrange and Eric Blake on public
> IRC channel mid-last year) for the above bug, since the actual bug has
> too many comments:
> 
> 
> https://kashyapc.fedorapeople.org/virt/openstack/nova-live-snapshot-bug-1334398-context-and-analysis.txt
> 
> The cause of it is almost narrowed down here by Dan in this comment
> 
> https://bugs.launchpad.net/nova/+bug/1334398/comments/27

Great thanks.
 
Yours Tony.


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

Re: [libvirt] [PATCH] Comment out variables/functions that are unused.

[Gary R Hook]

On 2/16/15 3:16 AM, Martin Kletzander wrote:

On Fri, Feb 13, 2015 at 08:18:53PM +, Gary R Hook wrote:

   Avoids complaints when the compiler is configured to "warn-unused".

   A few files contain unnecessary code that results in the compiler
   erroring out when -Wunused* options are used. Comment out the code
   until such time as it is needed.


---
src/libxl/libxl_conf.c   | 2 ++
tests/virnetsockettest.c | 8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 0555b91..f8db4d2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -305,7 +305,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
regmatch_t subs[4];
char *saveptr = NULL;
size_t i;
+/*
virArch hostarch = caps->host.arch;
+*/



How come you see this unused?  It is used about 100 lines later being
compared to VIR_ARCH_X86_64.


Ugh. You are right, and I can't believe I overlooked this. The problem 
is a bad Ubuntu patch that removes the reference to the variable, 
precipitating the compiler complaint. My apologies. I shall have to fix 
this in our build environment.




struct guest_arch guest_archs[32];
int nr_guest_archs = 0;
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index 5d91f26..988ab43 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -333,9 +333,10 @@ static int testSocketUNIXAddrs(const void *data
ATTRIBUTE_UNUSED)
return ret;
}

+/*
static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED)
{
-virNetSocketPtr csock = NULL; /* Client socket */
+virNetSocketPtr csock = NULL; / * Client socket * /


Ugly, guess what happens if someone was about uncomment the function.
If you used #if 0 around the function it'd be way easier, or I might
even consider removing that function, BUT...


Now I'm really annoyed. Same problem here: a badly implemented patch 
from Ubuntu (or Debian, but I don't really care which).



Let's leave this at: thank you for making me look just a bit further (as 
I should have to begin with) and thank you very much for your time. I am 
sorry to have wasted it.



--
Gary R Hook
Senior Kernel Engineer
NIMBOXX, Inc

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

[libvirt] [PATCH v2 2/4] remote: Implement the remote plumbing for virDomainGetIOThreads

[John Ferlan]

Implement the remote plumbing for virDomainGetIOThreads

Signed-off-by: John Ferlan 
---
 daemon/remote.c  | 74 +++-
 src/remote/remote_driver.c   | 81 +++-
 src/remote/remote_protocol.x | 28 +--
 src/remote_protocol-structs  | 19 +++
 src/rpc/gendispatch.pl   |  1 +
 5 files changed, 199 insertions(+), 4 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index d657a09..f41b1ca 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1,7 +1,7 @@
 /*
  * remote.c: handlers for RPC method calls
  *
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -2269,6 +2269,78 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 }
 
 static int
+remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_iothreads_info_args 
*args,
+ remote_domain_get_iothreads_info_ret *ret)
+{
+int rv = -1;
+size_t i;
+struct daemonClientPrivate *priv = 
virNetServerClientGetPrivateData(client);
+virDomainIOThreadsInfoPtr *info = NULL;
+virDomainPtr dom = NULL;
+remote_domain_iothreads_info *dst;
+int ninfo = 0;
+
+if (!priv->conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+goto cleanup;
+
+if ((ninfo = virDomainGetIOThreadsInfo(dom, &info, args->flags)) < 0)
+goto cleanup;
+
+if (ninfo > REMOTE_IOTHREADS_INFO_MAX) {
+virReportError(VIR_ERR_RPC,
+   _("Too many IOThreads in info: %d for limit %d"),
+   ninfo, REMOTE_IOTHREADS_INFO_MAX);
+goto cleanup;
+}
+
+if (ninfo) {
+if (VIR_ALLOC_N(ret->info.info_val, ninfo) < 0)
+goto cleanup;
+
+ret->info.info_len = ninfo;
+
+for (i = 0; i < ninfo; i++) {
+dst = &ret->info.info_val[i];
+dst->iothread_id = info[i]->iothread_id;
+
+/* No need to allocate/copy the cpumap if we make the reasonable
+ * assumption that unsigned char and char are the same size.
+ */
+dst->cpumap.cpumap_len = info[i]->cpumaplen;
+dst->cpumap.cpumap_val = (char *)info[i]->cpumap;
+info[i]->cpumap = NULL;
+}
+} else {
+ret->info.info_len = 0;
+ret->info.info_val = NULL;
+}
+
+ret->ret = ninfo;
+
+rv = 0;
+
+ cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+virObjectUnref(dom);
+if (ninfo >= 0)
+for (i = 0; i < ninfo; i++)
+virDomainIOThreadsInfoFree(info[i]);
+VIR_FREE(info);
+
+return rv;
+}
+
+static int
 remoteDispatchDomainMigratePrepare(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
virNetMessagePtr msg ATTRIBUTE_UNUSED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index d4fd658..944a590 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2,7 +2,7 @@
  * remote_driver.c: driver to provide access to libvirtd running
  *   on a remote machine
  *
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -2316,6 +2316,84 @@ remoteDomainGetVcpus(virDomainPtr domain,
 }
 
 static int
+remoteDomainGetIOThreadsInfo(virDomainPtr dom,
+ virDomainIOThreadsInfoPtr **info,
+ unsigned int flags)
+{
+int rv = -1;
+size_t i;
+struct private_data *priv = dom->conn->privateData;
+remote_domain_get_iothreads_info_args args;
+remote_domain_get_iothreads_info_ret ret;
+remote_domain_iothreads_info *src;
+virDomainIOThreadsInfoPtr *info_ret = NULL;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(&args.dom, dom);
+
+args.flags = flags;
+
+memset(&ret, 0, sizeof(ret));
+
+if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO,
+ (xdrproc_t)xdr_remote_domain_get_iothreads_info_args,
+ (char *)&args,
+ (xdrproc_t)xdr_remote_domain_get_iothreads_info_ret,
+ (char *)&ret) == -1)
+goto done;
+
+if (ret.info.info_len > REMOTE_IOTHREADS_INFO_MAX) {
+virRe

[libvirt] [PATCH v2 3/4] qemu: Implement the qemu driver fetch for IOThreads

[John Ferlan]

Depending on the flags passed, either attempt to return the active/live
IOThread data for the domain or the config data.

The active/live path will call into the Monitor in order to get the
IOThread data and then correlate the thread_id's returned from the
monitor to the currently running system/threads in order to ascertain
the affinity for each iothread_id.

The config path will map each of the configured IOThreads and return
any configured iothreadspin data

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 709f468..510ab41 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5541,6 +5541,222 @@ qemuDomainGetMaxVcpus(virDomainPtr dom)
  VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
+static int
+qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   virDomainIOThreadsInfoPtr **info)
+{
+qemuDomainObjPrivatePtr priv;
+qemuMonitorIOThreadsInfoPtr *iothreads = NULL;
+virDomainIOThreadsInfoPtr *info_ret = NULL;
+int niothreads = 0;
+int maxcpu, hostcpus, maplen;
+size_t i;
+int ret = -1;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("cannot list IOThreads for an inactive domain"));
+goto endjob;
+}
+
+priv = vm->privateData;
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("IOThreads not supported with this binary"));
+goto endjob;
+}
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+goto endjob;
+niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto endjob;
+if (niothreads < 0)
+goto endjob;
+
+/* Nothing to do */
+if (niothreads == 0) {
+ret = 0;
+goto endjob;
+}
+
+if ((hostcpus = nodeGetCPUCount()) < 0)
+goto endjob;
+
+maplen = VIR_CPU_MAPLEN(hostcpus);
+maxcpu = maplen * 8;
+if (maxcpu > hostcpus)
+maxcpu = hostcpus;
+
+if (VIR_ALLOC_N(info_ret, niothreads) < 0)
+goto endjob;
+
+for (i = 0; i < niothreads; i++) {
+virBitmapPtr map = NULL;
+unsigned char *tmpmap = NULL;
+int tmpmaplen = 0;
+
+if (VIR_ALLOC(info_ret[i]) < 0)
+goto endjob;
+
+if (virStrToLong_ui(iothreads[i]->name + strlen("iothread"), NULL, 10,
+&info_ret[i]->iothread_id) < 0)
+goto endjob;
+
+if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0)
+goto endjob;
+
+if (virProcessGetAffinity(iothreads[i]->thread_id, &map, maxcpu) < 0)
+goto endjob;
+
+virBitmapToData(map, &tmpmap, &tmpmaplen);
+if (tmpmaplen > maplen)
+tmpmaplen = maplen;
+memcpy(info_ret[i]->cpumap, tmpmap, tmpmaplen);
+info_ret[i]->cpumaplen = tmpmaplen;
+
+VIR_FREE(tmpmap);
+virBitmapFree(map);
+}
+
+*info = info_ret;
+info_ret = NULL;
+ret = niothreads;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+if (info_ret) {
+for (i = 0; i < niothreads; i++)
+virDomainIOThreadsInfoFree(info_ret[i]);
+VIR_FREE(info_ret);
+}
+
+return ret;
+}
+
+static int
+qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
+ virDomainIOThreadsInfoPtr **info)
+{
+virDomainIOThreadsInfoPtr *info_ret = NULL;
+virDomainVcpuPinDefPtr *iothreadspin_list;
+virBitmapPtr cpumask = NULL;
+unsigned char *cpumap;
+int maxcpu, hostcpus, maplen;
+size_t i, pcpu;
+bool pinned;
+int ret = -1;
+
+if (targetDef->iothreads == 0)
+return 0;
+
+if ((hostcpus = nodeGetCPUCount()) < 0)
+goto cleanup;
+
+maplen = VIR_CPU_MAPLEN(hostcpus);
+maxcpu = maplen * 8;
+if (maxcpu > hostcpus)
+maxcpu = hostcpus;
+
+if (VIR_ALLOC_N(info_ret, targetDef->iothreads) < 0)
+goto cleanup;
+
+for (i = 0; i < targetDef->iothreads; i++) {
+if (VIR_ALLOC(info_ret[i]) < 0)
+goto cleanup;
+
+/* IOThreads being counting at 1 */
+info_ret[i]->iothread_id = i + 1;
+
+if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0)
+goto cleanup;
+
+/* Initialize the cpumap */
+info_ret[i]->cpumaplen = maplen;
+memset(info_ret[i]->cpumap, 0xff, maplen);
+if (maxcpu % 8)
+info_ret[i]->cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
+}
+
+/* If iothreadspin setting exists, there are unu

[libvirt] [PATCH v2 0/4] Introduce display of IOThreads Information

[John Ferlan]

v1: http://www.redhat.com/archives/libvir-list/2015-February/msg00402.html

Changes since v1:
 * Remove 'thread_id' from public display
 * Add --live, --config, --current flags
   * Adjust patch 1 to accept/check the flags, modify comments
   * Adjust patch 3 to handle 'Live' or 'Config' options. Moved most of
 v1 patch code into the *Live function and add the *Config function
 to generate the IOThread config data
   * Adjust patch 4 to allow/check live/config options and set flags

John Ferlan (4):
  Implement public API for virDomainGetIOThreadsInfo
  remote: Implement the remote plumbing for virDomainGetIOThreads
  qemu: Implement the qemu driver fetch for IOThreads
  virsh: Add 'iothreads' command

 daemon/remote.c  |  74 -
 include/libvirt/libvirt-domain.h |  21 +++-
 src/driver-hypervisor.h  |   8 +-
 src/libvirt-domain.c |  75 +-
 src/libvirt_public.syms  |   6 ++
 src/qemu/qemu_driver.c   | 217 +++
 src/remote/remote_driver.c   |  81 ++-
 src/remote/remote_protocol.x |  28 -
 src/remote_protocol-structs  |  19 
 src/rpc/gendispatch.pl   |   1 +
 tools/virsh-domain.c |  94 +
 tools/virsh.pod  |  12 +++
 12 files changed, 629 insertions(+), 7 deletions(-)

-- 
2.1.0

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

[libvirt] [PATCH v2 1/4] Implement public API for virDomainGetIOThreadsInfo

[John Ferlan]

Add virDomainGetIOThreadsInfo in order to returned a list of
virDomainIOThreadsInfoPtr structures which list the IOThread ID
and the CPU Affinity map for each IOThread for domain. For an
active domain, the live data will be returned, while for an
inactive domain, the config data will be returned.  The API supports
either the --live or --config flag, but not both.

Also added virDomainIOThreadsInfoFree in order to free the cpumap
and the array entry structure.

Signed-off-by: John Ferlan 
---
 include/libvirt/libvirt-domain.h | 21 ++-
 src/driver-hypervisor.h  |  8 -
 src/libvirt-domain.c | 75 +++-
 src/libvirt_public.syms  |  6 
 4 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 4dbd7f5..60423fd 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4,7 +4,7 @@
  * Description: Provides APIs for the management of domains
  * Author: Daniel Veillard 
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -1568,6 +1568,25 @@ int virDomainGetEmulatorPinInfo 
(virDomainPtr domain,
  unsigned int flags);
 
 /**
+ * virIOThreadsInfo:
+ *
+ * The data structure for information about IOThreads in a domain
+ */
+typedef struct _virDomainIOThreadsInfo virDomainIOThreadsInfo;
+typedef virDomainIOThreadsInfo *virDomainIOThreadsInfoPtr;
+struct _virDomainIOThreadsInfo {
+unsigned int iothread_id;  /* IOThread ID */
+unsigned char *cpumap; /* CPU map for thread */
+int cpumaplen; /* cpumap size */
+};
+
+void virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr 
info);
+
+int  virDomainGetIOThreadsInfo(virDomainPtr domain,
+   virDomainIOThreadsInfoPtr 
**info,
+   unsigned int flags);
+
+/**
  * VIR_USE_CPU:
  * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT)
  * @cpu: the physical CPU number
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index a1198ad..2ce1a51 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1,7 +1,7 @@
 /*
  * driver-hypervisor.h: entry points for hypervisor drivers
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -381,6 +381,11 @@ typedef int
 (*virDrvDomainGetMaxVcpus)(virDomainPtr domain);
 
 typedef int
+(*virDrvDomainGetIOThreadsInfo)(virDomainPtr domain,
+virDomainIOThreadsInfoPtr **info,
+unsigned int flags);
+
+typedef int
 (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
 virSecurityLabelPtr seclabel);
 
@@ -1254,6 +1259,7 @@ struct _virHypervisorDriver {
 virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo;
 virDrvDomainGetVcpus domainGetVcpus;
 virDrvDomainGetMaxVcpus domainGetMaxVcpus;
+virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo;
 virDrvDomainGetSecurityLabel domainGetSecurityLabel;
 virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
 virDrvNodeGetSecurityModel nodeGetSecurityModel;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 492e90a..4a4e392 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1,7 +1,7 @@
 /*
  * libvirt-domain.c: entry points for virDomainPtr APIs
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -7891,6 +7891,79 @@ virDomainGetMaxVcpus(virDomainPtr domain)
 
 
 /**
+ * virDomainGetIOThreadsInfo:
+ * @dom: a domain object
+ * @info: pointer to an array of virDomainIOThreadsInfo structures (OUT)
+ * @flags: bitwise-OR of virDomainModificationImpact
+ * Must not be VIR_DOMAIN_AFFECT_LIVE and
+ * VIR_DOMAIN_AFFECT_CONFIG concurrently.
+ *
+ * Fetch IOThreads of an active domain including the cpumap information to
+ * determine on which CPU the IOThread has affinity to run.
+ *
+ * Returns the number of IOThreads or -1 in case of error.
+ * On success, the array of information is stored into @info. The caller is
+ * responsible for calling virDomainIOThreadsInfoFree() on each array element,
+ * then calling free() on @info. On error, @info is set to NULL.
+ */
+int
+virDomainGetIOThreadsInfo(virDomainPtr dom,
+  virDomainIOThreadsInfoPtr **info,
+ 

[libvirt] [PATCH v2 4/4] virsh: Add 'iothreads' command

[John Ferlan]

Add the 'iothreads' command to display IOThread Info data. Allow for
[--live] or [--config] options in order to display live or config data
for an active domain.

An active domain may return:

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

$ echo $?
0

For domains which don't have IOThreads the following is returned:

$ virsh iothreads $dom
error: No IOThreads found for the domain

$ echo $?
0

For domains which are not running the following is returned:

$ virsh iothreads $dom --live
error: Unable to get domain IOThreads information
error: Requested operation is not valid: cannot list IOThreads for an inactive 
domain

$ echo $?
1

Editing a running domains configuration and modifying the iothreadpin data
for thread 3 from nothing provided to setting a cpuset of '0-1' and then
displaying using --config could display:

$ virsh iothreads f18iothr --config
 IOThread ID CPU Affinity
 -
  1   2
  2   3
  3   0-1

$

Signed-off-by: John Ferlan 
---
 tools/virsh-domain.c | 94 
 tools/virsh.pod  | 12 +++
 2 files changed, 106 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2506b89..3b4480d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6797,6 +6797,94 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "iothreads" command
+ */
+static const vshCmdInfo info_iothreads[] = {
+{.name = "help",
+ .data = N_("view domain IOThreads")
+},
+{.name = "desc",
+ .data = N_("Returns basic information about the domain IOThreads.")
+},
+{.name = NULL}
+};
+static const vshCmdOptDef opts_iothreads[] = {
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("domain name, id or uuid")
+},
+{.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
+cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+bool current = vshCommandOptBool(cmd, "current");
+int niothreads = 0;
+virDomainIOThreadsInfoPtr *info;
+size_t i;
+int maxcpu;
+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 ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0)
+goto cleanup;
+
+if ((niothreads = virDomainGetIOThreadsInfo(dom, &info, flags)) < 0) {
+vshError(ctl, _("Unable to get domain IOThreads information"));
+goto cleanup;
+}
+
+if (niothreads == 0) {
+vshError(ctl, _("No IOThreads found for the domain"));
+goto cleanup;
+}
+
+vshPrintExtra(ctl, " %-15s %-15s\n",
+  _("IOThread ID"), _("CPU Affinity"));
+vshPrintExtra(ctl, "-\n");
+for (i = 0; i < niothreads; i++) {
+
+vshPrintExtra(ctl, " %-15u ", info[i]->iothread_id);
+ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen,
+ maxcpu, 0));
+vshPrint(ctl, "\n");
+virDomainIOThreadsInfoFree(info[i]);
+}
+VIR_FREE(info);
+
+ cleanup:
+virDomainFree(dom);
+return niothreads >= 0;
+}
+
+/*
  * "cpu-compare" command
  */
 static const vshCmdInfo info_cpu_compare[] = {
@@ -12697,6 +12785,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_inject_nmi,
  .flags = 0
 },
+{.name = "iothreads",
+ .handler = cmdIOThreadsInfo,
+ .opts = opts_iothreads,
+ .info = info_iothreads,
+ .flags = 0
+},
 {.name = "send-key",
  .handler = cmdSendKey,
  .opts = opts_send_key,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 50de32c..6d0a6fe 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1360,6 +1360,18 @@ If I<--timeout> is specified, the command gives up 
waiting for events
 after I have elapsed.   With I<--loop>, the command prints all
 events until a timeout or interrupt key.
 
+=item B I [[I<--live>] [I<--config>] | [I<--current>]]
+
+Display basic domain IOThreads information including the IOThread ID and
+the CPU Affinity for each IOThread.
+
+If I<--live> is specified, get the IOThreads data from the running guest. If
+the guest is not 

[libvirt] [PATCH 0/3] Fix memory ABI stability check issues

[Peter Krempa]

Note that this series applies only on top of the NUMA config unification series:
http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html

Please see individual patches for explanation

Peter Krempa (3):
  conf: ABI: Hugepage backing definition is not guest ABI
  conf: ABI: Memballoon setting is not guest ABI
  conf: numa: Check ABI stability of NUMA configuration

 src/conf/domain_conf.c   | 31 ++-
 src/conf/numa_conf.c | 37 +
 src/conf/numa_conf.h |  3 +++
 src/libvirt_private.syms |  1 +
 4 files changed, 43 insertions(+), 29 deletions(-)

-- 
2.2.2

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

[libvirt] [PATCH 1/3] conf: ABI: Hugepage backing definition is not guest ABI

[Peter Krempa]

The backing of the vm's memory isn't influencing the guest ABI thus
shouldn't be checked.
---
 src/conf/domain_conf.c | 24 
 1 file changed, 24 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fe73a5f..9a7060d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15969,30 +15969,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
dst->mem.cur_balloon, src->mem.cur_balloon);
 goto error;
 }
-if (src->mem.nhugepages != dst->mem.nhugepages) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain huge pages count %zu does not match 
source %zu"),
-   dst->mem.nhugepages, src->mem.nhugepages);
-goto error;
-}
-for (i = 0; i < src->mem.nhugepages; i++) {
-virDomainHugePagePtr src_huge = &src->mem.hugepages[i];
-virDomainHugePagePtr dst_huge = &dst->mem.hugepages[i];
-
-if (src_huge->size != dst_huge->size) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain huge page size %llu "
- "does not match source %llu"),
-   dst_huge->size, src_huge->size);
-goto error;
-}
-
-if (!virBitmapEqual(src_huge->nodemask, dst_huge->nodemask)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Target huge page nodemask does not match 
source"));
-goto error;
-}
-}

 if (src->vcpus != dst->vcpus) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
2.2.2

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

[libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI

[Peter Krempa]

The current balloon setting does not influence the ABI of the guest and
the driver should take the change just fine.
---
I personally don't think that this check is useful, but I'll happily drop the
patch if anybody thinks otherwise.

 src/conf/domain_conf.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a7060d..eee01b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15963,12 +15963,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
dst->mem.max_balloon, src->mem.max_balloon);
 goto error;
 }
-if (src->mem.cur_balloon != dst->mem.cur_balloon) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain current memory %lld does not match 
source %lld"),
-   dst->mem.cur_balloon, src->mem.cur_balloon);
-goto error;
-}

 if (src->vcpus != dst->vcpus) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
2.2.2

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

[libvirt] [PATCH 3/3] conf: numa: Check ABI stability of NUMA configuration

[Peter Krempa]

Add helper to compare initial sizes of indivitual NUMA nodes and the map
of belonging vCPUs. Other configuration is not ABI.
---
 src/conf/domain_conf.c   |  3 +++
 src/conf/numa_conf.c | 37 +
 src/conf/numa_conf.h |  3 +++
 src/libvirt_private.syms |  1 +
 4 files changed, 44 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eee01b6..15d4fe0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15964,6 +15964,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
 goto error;
 }

+if (!virDomainNumaCheckABIStability(src->numa, dst->numa))
+goto error;
+
 if (src->vcpus != dst->vcpus) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target domain vCPU count %d does not match source 
%d"),
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 0a6f902..70044d5 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -814,6 +814,43 @@ virDomainNumaNew(void)
 }


+bool
+virDomainNumaCheckABIStability(virDomainNumaPtr src,
+   virDomainNumaPtr tgt)
+{
+size_t i;
+
+if (virDomainNumaGetNodeCount(src) != virDomainNumaGetNodeCount(tgt)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target NUMA node count '%zu' doesn't match "
+ "source '%zu'"),
+   virDomainNumaGetNodeCount(tgt),
+   virDomainNumaGetNodeCount(src));
+return false;
+}
+
+for (i = 0; i < virDomainNumaGetNodeCount(src); i++) {
+if (src->mem_nodes[i].mem != tgt->mem_nodes[i].mem) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Size of target NUMA node %zu (%llu) doesn't "
+ "match source (%llu)"), i,
+   tgt->mem_nodes[i].mem, src->mem_nodes[i].mem);
+return false;
+}
+
+if (!virBitmapEqual(src->mem_nodes[i].cpumask,
+tgt->mem_nodes[i].cpumask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Processor mask of target NUMA node %zu doesn't "
+ "match source"), i);
+return false;
+}
+}
+
+return true;
+}
+
+
 size_t
 virDomainNumaGetNodeCount(virDomainNumaPtr numa)
 {
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 8b3c437..23265af 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -125,6 +125,9 @@ void virDomainNumaSetNodeMemorySize(virDomainNumaPtr numa,
 bool virDomainNumaEquals(virDomainNumaPtr n1,
  virDomainNumaPtr n2);

+bool virDomainNumaCheckABIStability(virDomainNumaPtr src,
+virDomainNumaPtr tgt);
+
 bool virDomainNumatuneHasPlacementAuto(virDomainNumaPtr numatune);

 bool virDomainNumatuneHasPerNodeBinding(virDomainNumaPtr numatune);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8988b61..c02e089 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -627,6 +627,7 @@ virNodeDeviceObjUnlock;


 # conf/numa_conf.h
+virDomainNumaCheckABIStability;
 virDomainNumaEquals;
 virDomainNumaFree;
 virDomainNumaGetNodeCount;
-- 
2.2.2

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

Re: [libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI

[Daniel P. Berrange]

On Mon, Feb 16, 2015 at 08:50:43PM +0100, Peter Krempa wrote:
> The current balloon setting does not influence the ABI of the guest and
> the driver should take the change just fine.
> ---
> I personally don't think that this check is useful, but I'll happily drop the
> patch if anybody thinks otherwise.
> 
>  src/conf/domain_conf.c | 6 --
>  1 file changed, 6 deletions(-)

Yes, you can change the balloon target on the fly, so this isn't something
we need to keep as stable across migration. The question though is whether
libvirt/QEMU will correctly handle the changed balloon setting during
migration. ie does libvirt correctly issue a 'balloon' monitor command on
the target host if it has changed in the target XML ?  If it does, then I
agree this check can go..

> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9a7060d..eee01b6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15963,12 +15963,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> dst->mem.max_balloon, src->mem.max_balloon);
>  goto error;
>  }
> -if (src->mem.cur_balloon != dst->mem.cur_balloon) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -   _("Target domain current memory %lld does not match 
> source %lld"),
> -   dst->mem.cur_balloon, src->mem.cur_balloon);
> -goto error;
> -}


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 05/24] conf: Move NUMA cell formatter to numa_conf

[Peter Krempa]

Move the code that formats the /domain/cpu/numa element to numa_conf as
it belongs there.
---
 src/conf/cpu_conf.c  | 29 -
 src/conf/numa_conf.c | 37 +
 src/conf/numa_conf.h |  1 +
 3 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 11ad5f4..28fbead 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -495,8 +495,11 @@ virCPUDefFormatBufFull(virBufferPtr buf,
   virArchToString(def->arch));
 if (virCPUDefFormatBuf(buf, def, updateCPU) < 0)
 return -1;
-virBufferAdjustIndent(buf, -2);

+if (virDomainNumaDefCPUFormat(buf, def) < 0)
+return -1;
+
+virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");

 return 0;
@@ -591,30 +594,6 @@ virCPUDefFormatBuf(virBufferPtr buf,
 }
 }

-if (def->ncells) {
-virBufferAddLit(buf, "\n");
-virBufferAdjustIndent(buf, 2);
-for (i = 0; i < def->ncells; i++) {
-virMemAccess memAccess = def->cells[i].memAccess;
-char *cpustr = NULL;
-
-if (!(cpustr = virBitmapFormat(def->cells[i].cpumask)))
-return -1;
-
-virBufferAddLit(buf, "cells[i].mem);
-virBufferAddLit(buf, " unit='KiB'");
-if (memAccess)
-virBufferAsprintf(buf, " memAccess='%s'",
-  virMemAccessTypeToString(memAccess));
-virBufferAddLit(buf, "/>\n");
-VIR_FREE(cpustr);
-}
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
-}
 return 0;
 }

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 965d67b..d8f1739 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -777,3 +777,40 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 VIR_FREE(tmp);
 return ret;
 }
+
+
+int
+virDomainNumaDefCPUFormat(virBufferPtr buf,
+  virCPUDefPtr def)
+{
+virMemAccess memAccess;
+char *cpustr;
+size_t i;
+
+if (def->ncells == 0)
+return 0;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+for (i = 0; i < def->ncells; i++) {
+memAccess = def->cells[i].memAccess;
+
+if (!(cpustr = virBitmapFormat(def->cells[i].cpumask)))
+return -1;
+
+virBufferAddLit(buf, "cells[i].mem);
+virBufferAddLit(buf, " unit='KiB'");
+if (memAccess)
+virBufferAsprintf(buf, " memAccess='%s'",
+  virMemAccessTypeToString(memAccess));
+virBufferAddLit(buf, "/>\n");
+VIR_FREE(cpustr);
+}
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+
+return 0;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 276d25a..9202355 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -114,5 +114,6 @@ bool 
virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune,
  virBitmapPtr auto_nodeset);

 int virDomainNumaDefCPUParseXML(virCPUDefPtr def, xmlXPathContextPtr ctxt);
+int virDomainNumaDefCPUFormat(virBufferPtr buf, virCPUDefPtr def);

 #endif /* __NUMA_CONF_H__ */
-- 
2.2.2

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

[libvirt] [PATCH 08/24] conf: numa: Recalculate rather than remember total NUMA cpu count

[Peter Krempa]

It's easier to recalculate the number in the one place it's used as
having a separate varialbe to track it. It will also help with moving
the NUMA code to the separate module.
---
 src/conf/cpu_conf.c|  1 -
 src/conf/cpu_conf.h|  1 -
 src/conf/domain_conf.c |  2 +-
 src/conf/numa_conf.c   | 23 +--
 src/conf/numa_conf.h   |  3 +++
 tests/testutilsqemu.c  |  1 -
 6 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 4923618..0e3ce26 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -155,7 +155,6 @@ virCPUDefCopy(const virCPUDef *cpu)
 if (!copy->cells[i].cpumask)
 goto error;
 }
-copy->cells_cpus = cpu->cells_cpus;
 }

 return copy;
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 0163384..d2563e2 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -117,7 +117,6 @@ struct _virCPUDef {
 virCPUFeatureDefPtr features;
 size_t ncells;
 virCellDefPtr cells;
-unsigned int cells_cpus;
 };


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c9a65e1..870efc9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13501,7 +13501,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;

 if (def->cpu &&
-def->cpu->cells_cpus > def->maxvcpus) {
+virDomainNumaGetCPUCountTotal(def->cpu) > def->maxvcpus) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Number of CPUs in  exceeds the"
  "  count"));
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index eea4172..41a7e01 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -713,7 +713,7 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 def->ncells = n;

 for (i = 0; i < n; i++) {
-int rc, ncpus = 0;
+int rc;
 unsigned int cur_cell = i;

 /* cells are in order of parsing or explicitly numbered */
@@ -748,12 +748,10 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 goto cleanup;
 }

-ncpus = virBitmapParse(tmp, 0, &def->cells[cur_cell].cpumask,
-   VIR_DOMAIN_CPUMASK_LEN);
-
-if (ncpus <= 0)
+if (virBitmapParse(tmp, 0, &def->cells[cur_cell].cpumask,
+   VIR_DOMAIN_CPUMASK_LEN) <= 0)
 goto cleanup;
-def->cells_cpus += ncpus;
+
 VIR_FREE(tmp);

 ctxt->node = nodes[i];
@@ -819,3 +817,16 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,

 return 0;
 }
+
+
+unsigned int
+virDomainNumaGetCPUCountTotal(virCPUDefPtr numa)
+{
+size_t i;
+unsigned int ret = 0;
+
+for (i = 0; i < numa->ncells; i++)
+ret += virBitmapCountBits(numa->cells[i].cpumask);
+
+return ret;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index ca89e05..fa6b89b 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -125,4 +125,7 @@ bool virDomainNumatuneNodesetIsAvailable(virDomainNumaPtr 
numatune,
 int virDomainNumaDefCPUParseXML(virCPUDefPtr def, xmlXPathContextPtr ctxt);
 int virDomainNumaDefCPUFormat(virBufferPtr buf, virCPUDefPtr def);

+unsigned int virDomainNumaGetCPUCountTotal(virCPUDefPtr numa);
+
+
 #endif /* __NUMA_CONF_H__ */
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 64f709a..4a3df8a 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -244,7 +244,6 @@ virCapsPtr testQemuCapsInit(void)
 host_cpu_features,  /* features */
 0,  /* ncells */
 NULL,   /* cells */
-0,  /* cells_cpus */
 };

 if ((caps = virCapabilitiesNew(host_cpu.arch,
-- 
2.2.2

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

[libvirt] [PATCH 10/24] conf: numa: Reformat virDomainNumatuneParseXML

[Peter Krempa]

Make collapse few of the conditions so that the program flow is more
clear.
---
 src/conf/numa_conf.c | 32 
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 391ef15..18c21d5 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -245,32 +245,24 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,
 goto cleanup;
 }

-tmp = virXMLPropString(node, "mode");
-if (tmp) {
-mode = virDomainNumatuneMemModeTypeFromString(tmp);
-if (mode < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Unsupported NUMA memory tuning mode '%s'"),
-   tmp);
-goto cleanup;
-}
+if ((tmp = virXMLPropString(node, "mode")) &&
+(mode = virDomainNumatuneMemModeTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unsupported NUMA memory tuning mode '%s'"), tmp);
+goto cleanup;
 }
 VIR_FREE(tmp);

-tmp = virXMLPropString(node, "placement");
-if (tmp) {
-placement = virDomainNumatunePlacementTypeFromString(tmp);
-if (placement < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Unsupported NUMA memory placement mode '%s'"),
-   tmp);
-goto cleanup;
-}
+if ((tmp = virXMLPropString(node, "placement")) &&
+(placement = virDomainNumatunePlacementTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unsupported NUMA memory placement mode '%s'"), tmp);
+goto cleanup;
 }
 VIR_FREE(tmp);

-tmp = virXMLPropString(node, "nodeset");
-if (tmp && virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
+if ((tmp = virXMLPropString(node, "nodeset")) &&
+virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
 goto cleanup;
 VIR_FREE(tmp);

-- 
2.2.2

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

[libvirt] [PATCH 04/24] conf: numa: Don't duplicate NUMA cell cpumask

[Peter Krempa]

The mask was stored both as a bitmap and as a string. The string is used
for XML output only. Remove the string, as it can be reconstructed from
the bitmap.

The test change is necessary as the bitmap formatter doesn't "optimize"
using the '^' operator.
---
 src/conf/cpu_conf.c| 14 +++---
 src/conf/cpu_conf.h|  1 -
 src/conf/numa_conf.c   |  3 +--
 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml   |  2 +-
 .../qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml |  2 +-
 5 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 98b309b..11ad5f4 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -86,10 +86,8 @@ virCPUDefFree(virCPUDefPtr def)

 virCPUDefFreeModel(def);

-for (i = 0; i < def->ncells; i++) {
+for (i = 0; i < def->ncells; i++)
 virBitmapFree(def->cells[i].cpumask);
-VIR_FREE(def->cells[i].cpustr);
-}
 VIR_FREE(def->cells);
 VIR_FREE(def->vendor_id);

@@ -162,9 +160,6 @@ virCPUDefCopy(const virCPUDef *cpu)

 if (!copy->cells[i].cpumask)
 goto error;
-
-if (VIR_STRDUP(copy->cells[i].cpustr, cpu->cells[i].cpustr) < 0)
-goto error;
 }
 copy->cells_cpus = cpu->cells_cpus;
 }
@@ -601,16 +596,21 @@ virCPUDefFormatBuf(virBufferPtr buf,
 virBufferAdjustIndent(buf, 2);
 for (i = 0; i < def->ncells; i++) {
 virMemAccess memAccess = def->cells[i].memAccess;
+char *cpustr = NULL;
+
+if (!(cpustr = virBitmapFormat(def->cells[i].cpumask)))
+return -1;

 virBufferAddLit(buf, "cells[i].cpustr);
+virBufferAsprintf(buf, " cpus='%s'", cpustr);
 virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem);
 virBufferAddLit(buf, " unit='KiB'");
 if (memAccess)
 virBufferAsprintf(buf, " memAccess='%s'",
   virMemAccessTypeToString(memAccess));
 virBufferAddLit(buf, "/>\n");
+VIR_FREE(cpustr);
 }
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index e201656..d6efba7 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -104,7 +104,6 @@ typedef struct _virCellDef virCellDef;
 typedef virCellDef *virCellDefPtr;
 struct _virCellDef {
 virBitmapPtr cpumask; /* CPUs that are part of this node */
-char *cpustr; /* CPUs stored in string form for dumpxml */
 unsigned long long mem; /* Node memory in kB */
 virMemAccess memAccess;
 };
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index c50369d..965d67b 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -749,8 +749,7 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 if (ncpus <= 0)
 goto cleanup;
 def->cells_cpus += ncpus;
-
-def->cells[cur_cell].cpustr = tmp;
+VIR_FREE(tmp);

 ctxt->node = nodes[i];
 if (virDomainParseMemory("./@memory", "./@unit", ctxt,
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
index 8912017..73dfdf5 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
@@ -17,7 +17,7 @@
 
   
   
-  
+  
 
   
   
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
index ffc57cf..7542125 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
@@ -17,7 +17,7 @@
 
   
   
-  
+  
 
   
   
-- 
2.2.2

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

[libvirt] [PATCH 23/24] conf: numa: Add setter/getter for NUMA node memory size

[Peter Krempa]

Add the helpers and refactor places where the value is accessed without
them.
---
 src/conf/numa_conf.c | 20 +++-
 src/conf/numa_conf.h |  7 +++
 src/libvirt_private.syms |  2 ++
 src/qemu/qemu_command.c  | 10 ++
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 4906687..ee7e65d 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -777,7 +777,8 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
 virBufferAddLit(buf, "cells[i].mem);
+virBufferAsprintf(buf, " memory='%llu'",
+  virDomainNumaGetNodeMemorySize(def, i));
 virBufferAddLit(buf, " unit='KiB'");
 if (memAccess)
 virBufferAsprintf(buf, " memAccess='%s'",
@@ -840,3 +841,20 @@ virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa,
 {
 return numa->cells[node].memAccess;
 }
+
+
+unsigned long long
+virDomainNumaGetNodeMemorySize(virCPUDefPtr numa,
+   size_t node)
+{
+return numa->cells[node].mem;
+}
+
+
+void
+virDomainNumaSetNodeMemorySize(virCPUDefPtr numa,
+   size_t node,
+   unsigned long long size)
+{
+numa->cells[node].mem = size;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 0ea2c93..eadab43 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -91,6 +91,9 @@ virBitmapPtr virDomainNumaGetNodeCpumask(virCPUDefPtr numa,
  size_t node);
 virNumaMemAccess virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa,
   size_t node);
+unsigned long long virDomainNumaGetNodeMemorySize(virCPUDefPtr numa,
+  size_t node);
+

 /*
  * Formatters
@@ -114,6 +117,10 @@ int virDomainNumatuneSet(virDomainNumaPtr numa,
  virBitmapPtr nodeset)
 ATTRIBUTE_NONNULL(1);

+void virDomainNumaSetNodeMemorySize(virCPUDefPtr numa,
+size_t node,
+unsigned long long size);
+
 /*
  * Other accessors
  */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1f1ce14..8988b61 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -632,7 +632,9 @@ virDomainNumaFree;
 virDomainNumaGetNodeCount;
 virDomainNumaGetNodeCpumask;
 virDomainNumaGetNodeMemoryAccessMode;
+virDomainNumaGetNodeMemorySize;
 virDomainNumaNew;
+virDomainNumaSetNodeMemorySize;
 virDomainNumatuneFormatNodeset;
 virDomainNumatuneFormatXML;
 virDomainNumatuneGetMode;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 05545ee..a69d004 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4745,7 +4745,8 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
 if (virAsprintf(&alias, "ram-node%zu", cell) < 0)
 goto cleanup;

-if ((rc = qemuBuildMemoryBackendStr(def->cpu->cells[cell].mem, 0, cell,
+if ((rc = 
qemuBuildMemoryBackendStr(virDomainNumaGetNodeMemorySize(def->cpu, cell),
+0, cell,
 NULL, auto_nodeset,
 def, qemuCaps, cfg,
 &backendType, &props, false)) < 0)
@@ -7173,8 +7174,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 /* using of -numa memdev= cannot be combined with -numa mem=, thus we
  * need to check which approach to use */
 for (i = 0; i < ncells; i++) {
-unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
-def->cpu->cells[i].mem = cellmem * 1024;
+unsigned long long cellmem = virDomainNumaGetNodeMemorySize(def->cpu, 
i);
+virDomainNumaSetNodeMemorySize(def->cpu, i, VIR_ROUND_UP(cellmem, 
1024));

 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
@@ -7224,7 +7225,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 if (needBackend)
 virBufferAsprintf(&buf, ",memdev=ram-node%zu", i);
 else
-virBufferAsprintf(&buf, ",mem=%llu", def->cpu->cells[i].mem / 
1024);
+virBufferAsprintf(&buf, ",mem=%llu",
+  virDomainNumaGetNodeMemorySize(def->cpu, i) / 
1024);

 virCommandAddArgBuffer(cmd, &buf);
 }
-- 
2.2.2

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

[libvirt] [PATCH 03/24] conf: Refactor virDomainNumaDefCPUParseXML

[Peter Krempa]

Rewrite the function to save a few local variables and reorder the code
to make more sense.

Additionally the ncells_max member of the virCPUDef structure is used
only for tracking alocation when parsing the numa definition, which can
be avoided by switching to VIR_ALLOC_N as the array is not resized
after initial allocation.
---
 src/conf/cpu_conf.c   |   1 -
 src/conf/cpu_conf.h   |   1 -
 src/conf/numa_conf.c  | 129 +++---
 tests/testutilsqemu.c |   1 -
 4 files changed, 58 insertions(+), 74 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index f14b37a..98b309b 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -154,7 +154,6 @@ virCPUDefCopy(const virCPUDef *cpu)
 if (cpu->ncells) {
 if (VIR_ALLOC_N(copy->cells, cpu->ncells) < 0)
 goto error;
-copy->ncells_max = copy->ncells = cpu->ncells;

 for (i = 0; i < cpu->ncells; i++) {
 copy->cells[i].mem = cpu->cells[i].mem;
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 46fce25..e201656 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -127,7 +127,6 @@ struct _virCPUDef {
 size_t nfeatures_max;
 virCPUFeatureDefPtr features;
 size_t ncells;
-size_t ncells_max;
 virCellDefPtr cells;
 unsigned int cells_cpus;
 };
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index e36a4be..c50369d 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -688,45 +688,36 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 {
 xmlNodePtr *nodes = NULL;
 xmlNodePtr oldNode = ctxt->node;
+char *tmp = NULL;
 int n;
 size_t i;
 int ret = -1;

-if (virXPathNode("/domain/cpu/numa[1]", ctxt)) {
-VIR_FREE(nodes);
+/* check if NUMA definition is present */
+if (!virXPathNode("/domain/cpu/numa[1]", ctxt))
+return 0;

-n = virXPathNodeSet("/domain/cpu/numa[1]/cell", ctxt, &nodes);
-if (n <= 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("NUMA topology defined without NUMA cells"));
-goto error;
-}
+if ((n = virXPathNodeSet("/domain/cpu/numa[1]/cell", ctxt, &nodes)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("NUMA topology defined without NUMA cells"));
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(def->cells, n) < 0)
+goto cleanup;
+def->ncells = n;

-if (VIR_RESIZE_N(def->cells, def->ncells_max,
- def->ncells, n) < 0)
-goto error;
-
-def->ncells = n;
-
-for (i = 0; i < n; i++) {
-char *cpus, *memAccessStr;
-int rc, ncpus = 0;
-unsigned int cur_cell;
-char *tmp = NULL;
-
-tmp = virXMLPropString(nodes[i], "id");
-if (!tmp) {
-cur_cell = i;
-} else {
-rc  = virStrToLong_ui(tmp, NULL, 10, &cur_cell);
-if (rc == -1) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid 'id' attribute in NUMA cell: 
%s"),
-   tmp);
-VIR_FREE(tmp);
-goto error;
-}
-VIR_FREE(tmp);
+for (i = 0; i < n; i++) {
+int rc, ncpus = 0;
+unsigned int cur_cell = i;
+
+/* cells are in order of parsing or explicitly numbered */
+if ((tmp = virXMLPropString(nodes[i], "id"))) {
+if (virStrToLong_uip(tmp, NULL, 10, &cur_cell) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid 'id' attribute in NUMA cell: '%s'"),
+   tmp);
+goto cleanup;
 }

 if (cur_cell >= n) {
@@ -734,60 +725,56 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
_("Exactly one 'cell' element per guest "
  "NUMA cell allowed, non-contiguous ranges or "
  "ranges not starting from 0 are not 
allowed"));
-goto error;
-}
-
-if (def->cells[cur_cell].cpustr) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Duplicate NUMA cell info for cell id '%u'"),
-   cur_cell);
-goto error;
+goto cleanup;
 }
+}
+VIR_FREE(tmp);

-cpus = virXMLPropString(nodes[i], "cpus");
-if (!cpus) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("Missing 'cpus' attribute in NUMA cell"));
-goto error;
-}
-def->cells[cur_cell].cpustr = cpus;
+if (def->cells[cur_cell].cpumask) {
+virReportError(VIR_ERR_XML_ERROR,
+

[libvirt] [PATCH 24/24] conf: Move all NUMA configuration to virDomainNuma

[Peter Krempa]

For historical reasons data regarding NUMA configuration were split
between the CPU definition and numatune. We cannot do anything about the
XML still being split, but we certainly can at least store the relevant
data in one place.

This patch moves the NUMA stuff to the right place.
---
 src/conf/cpu_conf.c | 26 +++
 src/conf/cpu_conf.h | 12 +++--
 src/conf/domain_conf.c  |  8 +++---
 src/conf/numa_conf.c| 67 +++--
 src/conf/numa_conf.h| 20 +++
 src/cpu/cpu.c   |  2 +-
 src/qemu/qemu_command.c | 20 +++
 tests/cputest.c |  2 +-
 tests/testutilsqemu.c   |  2 --
 9 files changed, 63 insertions(+), 96 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 0e3ce26..4e33934 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -73,16 +73,11 @@ virCPUDefFreeModel(virCPUDefPtr def)
 void
 virCPUDefFree(virCPUDefPtr def)
 {
-size_t i;
-
 if (!def)
 return;

 virCPUDefFreeModel(def);

-for (i = 0; i < def->ncells; i++)
-virBitmapFree(def->cells[i].cpumask);
-VIR_FREE(def->cells);
 VIR_FREE(def->vendor_id);

 VIR_FREE(def);
@@ -126,7 +121,6 @@ virCPUDefPtr
 virCPUDefCopy(const virCPUDef *cpu)
 {
 virCPUDefPtr copy;
-size_t i;

 if (!cpu || VIR_ALLOC(copy) < 0)
 return NULL;
@@ -143,20 +137,6 @@ virCPUDefCopy(const virCPUDef *cpu)
 if (virCPUDefCopyModel(copy, cpu, false) < 0)
 goto error;

-if (cpu->ncells) {
-if (VIR_ALLOC_N(copy->cells, cpu->ncells) < 0)
-goto error;
-
-for (i = 0; i < cpu->ncells; i++) {
-copy->cells[i].mem = cpu->cells[i].mem;
-
-copy->cells[i].cpumask = virBitmapNewCopy(cpu->cells[i].cpumask);
-
-if (!copy->cells[i].cpumask)
-goto error;
-}
-}
-
 return copy;

  error:
@@ -429,11 +409,12 @@ virCPUDefParseXML(xmlNodePtr node,

 char *
 virCPUDefFormat(virCPUDefPtr def,
+virDomainNumaPtr numa,
 bool updateCPU)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;

-if (virCPUDefFormatBufFull(&buf, def, updateCPU) < 0)
+if (virCPUDefFormatBufFull(&buf, def, numa, updateCPU) < 0)
 goto cleanup;

 if (virBufferCheckError(&buf) < 0)
@@ -450,6 +431,7 @@ virCPUDefFormat(virCPUDefPtr def,
 int
 virCPUDefFormatBufFull(virBufferPtr buf,
virCPUDefPtr def,
+   virDomainNumaPtr numa,
bool updateCPU)
 {
 if (!def)
@@ -489,7 +471,7 @@ virCPUDefFormatBufFull(virBufferPtr buf,
 if (virCPUDefFormatBuf(buf, def, updateCPU) < 0)
 return -1;

-if (virDomainNumaDefCPUFormat(buf, def) < 0)
+if (virDomainNumaDefCPUFormat(buf, numa) < 0)
 return -1;

 virBufferAdjustIndent(buf, -2);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index d2563e2..705ba6d 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -29,6 +29,7 @@
 # include "virxml.h"
 # include "virbitmap.h"
 # include "virarch.h"
+# include "numa_conf.h"

 # define VIR_CPU_VENDOR_ID_LENGTH 12

@@ -90,13 +91,6 @@ struct _virCPUFeatureDef {
 int policy; /* enum virCPUFeaturePolicy */
 };

-typedef struct _virCellDef virCellDef;
-typedef virCellDef *virCellDefPtr;
-struct _virCellDef {
-virBitmapPtr cpumask; /* CPUs that are part of this node */
-unsigned long long mem; /* Node memory in kB */
-int memAccess; /* virNumaMemAccess */
-};

 typedef struct _virCPUDef virCPUDef;
 typedef virCPUDef *virCPUDefPtr;
@@ -115,8 +109,6 @@ struct _virCPUDef {
 size_t nfeatures;
 size_t nfeatures_max;
 virCPUFeatureDefPtr features;
-size_t ncells;
-virCellDefPtr cells;
 };


@@ -145,6 +137,7 @@ virCPUDefIsEqual(virCPUDefPtr src,

 char *
 virCPUDefFormat(virCPUDefPtr def,
+virDomainNumaPtr numa,
 bool updateCPU);

 int
@@ -154,6 +147,7 @@ virCPUDefFormatBuf(virBufferPtr buf,
 int
 virCPUDefFormatBufFull(virBufferPtr buf,
virCPUDefPtr def,
+   virDomainNumaPtr numa,
bool updateCPU);

 int
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d34b9c4..fe73a5f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13517,11 +13517,10 @@ virDomainDefParseXML(xmlDocPtr xml,

 }

-if (virDomainNumaDefCPUParseXML(def->cpu, ctxt) < 0)
+if (virDomainNumaDefCPUParseXML(def->numa, ctxt) < 0)
 goto error;

-if (def->cpu &&
-virDomainNumaGetCPUCountTotal(def->cpu) > def->maxvcpus) {
+if (virDomainNumaGetCPUCountTotal(def->numa) > def->maxvcpus) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Number of CPUs in  exceeds the"
  "  count"));
@@ -13531,7 +13530,6 @@ virDomainDefParseXML(xmlDocPtr xml,
 if (virDomainNumatuneParseXML(def->numa,
 

[libvirt] [PATCH 07/24] conf: Move enum virMemAccess to the NUMA code and rename it

[Peter Krempa]

Name it virNumaMemAccess and add it to conf/numa_conf.[ch]

Note that to avoid a circular dependency the type of the NUMA cell
memAccess variable was changed to int. It will be turned back later
after the circular dependency will not exist.
---
 src/conf/cpu_conf.c |  6 --
 src/conf/cpu_conf.h | 12 +---
 src/conf/numa_conf.c| 11 ---
 src/conf/numa_conf.h|  9 +
 src/qemu/qemu_command.c | 10 +-
 5 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 28fbead..4923618 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -56,12 +56,6 @@ VIR_ENUM_IMPL(virCPUFeaturePolicy, VIR_CPU_FEATURE_LAST,
   "disable",
   "forbid")

-VIR_ENUM_IMPL(virMemAccess, VIR_MEM_ACCESS_LAST,
-  "default",
-  "shared",
-  "private")
-
-
 void ATTRIBUTE_NONNULL(1)
 virCPUDefFreeModel(virCPUDefPtr def)
 {
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index d6efba7..0163384 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -83,16 +83,6 @@ typedef enum {

 VIR_ENUM_DECL(virCPUFeaturePolicy)

-typedef enum {
-VIR_MEM_ACCESS_DEFAULT,
-VIR_MEM_ACCESS_SHARED,
-VIR_MEM_ACCESS_PRIVATE,
-
-VIR_MEM_ACCESS_LAST,
-} virMemAccess;
-
-VIR_ENUM_DECL(virMemAccess)
-
 typedef struct _virCPUFeatureDef virCPUFeatureDef;
 typedef virCPUFeatureDef *virCPUFeatureDefPtr;
 struct _virCPUFeatureDef {
@@ -105,7 +95,7 @@ typedef virCellDef *virCellDefPtr;
 struct _virCellDef {
 virBitmapPtr cpumask; /* CPUs that are part of this node */
 unsigned long long mem; /* Node memory in kB */
-virMemAccess memAccess;
+int memAccess; /* virNumaMemAccess */
 };

 typedef struct _virCPUDef virCPUDef;
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index bcb8023..eea4172 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -43,6 +43,11 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
   "static",
   "auto");

+VIR_ENUM_IMPL(virNumaMemAccess, VIR_NUMA_MEM_ACCESS_LAST,
+  "default",
+  "shared",
+  "private");
+
 typedef struct _virDomainNumaNode virDomainNumaNode;
 typedef virDomainNumaNode *virDomainNumaNodePtr;

@@ -757,7 +762,7 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 goto cleanup;

 if ((tmp = virXMLPropString(nodes[i], "memAccess"))) {
-if ((rc = virMemAccessTypeFromString(tmp)) <= 0) {
+if ((rc = virNumaMemAccessTypeFromString(tmp)) <= 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid 'memAccess' attribute value '%s'"),
tmp);
@@ -783,7 +788,7 @@ int
 virDomainNumaDefCPUFormat(virBufferPtr buf,
   virCPUDefPtr def)
 {
-virMemAccess memAccess;
+virNumaMemAccess memAccess;
 char *cpustr;
 size_t i;

@@ -805,7 +810,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
 virBufferAddLit(buf, " unit='KiB'");
 if (memAccess)
 virBufferAsprintf(buf, " memAccess='%s'",
-  virMemAccessTypeToString(memAccess));
+  virNumaMemAccessTypeToString(memAccess));
 virBufferAddLit(buf, "/>\n");
 VIR_FREE(cpustr);
 }
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 0adeaa4..ca89e05 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -46,6 +46,15 @@ typedef enum {
 VIR_ENUM_DECL(virDomainNumatunePlacement)
 VIR_ENUM_DECL(virDomainNumatuneMemMode)

+typedef enum {
+VIR_NUMA_MEM_ACCESS_DEFAULT,
+VIR_NUMA_MEM_ACCESS_SHARED,
+VIR_NUMA_MEM_ACCESS_PRIVATE,
+
+VIR_NUMA_MEM_ACCESS_LAST
+} virNumaMemAccess;
+
+VIR_ENUM_DECL(virNumaMemAccess)

 void virDomainNumaFree(virDomainNumaPtr numa);

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c4ae596..b6fca9c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4558,7 +4558,7 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 virDomainHugePagePtr hugepage = NULL;
 virDomainNumatuneMemMode mode;
 const long system_page_size = virGetSystemPageSizeKB();
-virMemAccess memAccess = def->cpu->cells[guestNode].memAccess;
+virNumaMemAccess memAccess = def->cpu->cells[guestNode].memAccess;
 size_t i;
 char *mem_path = NULL;
 virBitmapPtr nodemask = NULL;
@@ -4651,18 +4651,18 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 goto cleanup;

 switch (memAccess) {
-case VIR_MEM_ACCESS_SHARED:
+case VIR_NUMA_MEM_ACCESS_SHARED:
 if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0)
 goto cleanup;
 break;

-case VIR_MEM_ACCESS_PRIVATE:
+case VIR_NUMA_MEM_ACCESS_PRIVATE:
 if (virJSONValueObjectAdd(props, "b:share", false, NULL) < 0)
 goto cleanup;

[libvirt] [PATCH 22/24] conf: numa: Add accessor to NUMA node's memory access mode

[Peter Krempa]

---
 src/conf/numa_conf.c | 10 +-
 src/conf/numa_conf.h |  2 ++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  |  5 +++--
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index acda415..4906687 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -769,7 +769,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
 virBufferAddLit(buf, "\n");
 virBufferAdjustIndent(buf, 2);
 for (i = 0; i < ncells; i++) {
-memAccess = def->cells[i].memAccess;
+memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i);

 if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i
 return -1;
@@ -832,3 +832,11 @@ virDomainNumaGetNodeCpumask(virCPUDefPtr numa,
 {
 return numa->cells[node].cpumask;
 }
+
+
+virNumaMemAccess
+virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa,
+ size_t node)
+{
+return numa->cells[node].memAccess;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index ab88ab7..0ea2c93 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -89,6 +89,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr 
numatune,
 size_t virDomainNumaGetNodeCount(virCPUDefPtr numa);
 virBitmapPtr virDomainNumaGetNodeCpumask(virCPUDefPtr numa,
  size_t node);
+virNumaMemAccess virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa,
+  size_t node);

 /*
  * Formatters
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 770803b..1f1ce14 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -631,6 +631,7 @@ virDomainNumaEquals;
 virDomainNumaFree;
 virDomainNumaGetNodeCount;
 virDomainNumaGetNodeCpumask;
+virDomainNumaGetNodeMemoryAccessMode;
 virDomainNumaNew;
 virDomainNumatuneFormatNodeset;
 virDomainNumatuneFormatXML;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d276da3..05545ee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4558,7 +4558,8 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 virDomainHugePagePtr hugepage = NULL;
 virDomainNumatuneMemMode mode;
 const long system_page_size = virGetSystemPageSizeKB();
-virNumaMemAccess memAccess = def->cpu->cells[guestNode].memAccess;
+virNumaMemAccess memAccess = 
virDomainNumaGetNodeMemoryAccessMode(def->cpu, guestNode);
+
 size_t i;
 char *mem_path = NULL;
 virBitmapPtr nodemask = NULL;
@@ -7185,7 +7186,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 if (rc == 0)
 needBackend = true;
 } else {
-if (def->cpu->cells[i].memAccess) {
+if (virDomainNumaGetNodeMemoryAccessMode(def->cpu, i)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Shared memory mapping is not supported "
  "with this QEMU"));
-- 
2.2.2

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

[libvirt] [PATCH 06/24] conf: numa: Rename virDomainNumatune to virDomainNuma

[Peter Krempa]

The structure will gradually become the only place for NUMA related
config, thus rename it appropriately.
---
 src/conf/domain_conf.c| 10 +++---
 src/conf/domain_conf.h|  2 +-
 src/conf/numa_conf.c  | 78 +--
 src/conf/numa_conf.h  | 34 +--
 src/libvirt_private.syms  |  4 +--
 src/lxc/lxc_cgroup.c  |  4 +--
 src/lxc/lxc_controller.c  |  6 ++--
 src/lxc/lxc_native.c  |  2 +-
 src/parallels/parallels_sdk.c |  2 +-
 src/qemu/qemu_cgroup.c| 12 +++
 src/qemu/qemu_command.c   |  8 ++---
 src/qemu/qemu_driver.c| 20 +--
 src/qemu/qemu_process.c   |  4 +--
 13 files changed, 93 insertions(+), 93 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 06ed0fd..c9a65e1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2249,7 +2249,7 @@ void virDomainDefFree(virDomainDefPtr def)
 virBitmapFree(def->cputune.iothreadsched[i].ids);
 VIR_FREE(def->cputune.iothreadsched);

-virDomainNumatuneFree(def->numatune);
+virDomainNumaFree(def->numa);

 virSysinfoDefFree(def->sysinfo);

@@ -13508,14 +13508,14 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }

-if (virDomainNumatuneParseXML(&def->numatune,
+if (virDomainNumatuneParseXML(&def->numa,
   def->placement_mode ==
   VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
   def->cpu ? def->cpu->ncells : 0,
   ctxt) < 0)
 goto error;

-if (virDomainNumatuneHasPlacementAuto(def->numatune) &&
+if (virDomainNumatuneHasPlacementAuto(def->numa) &&
 !def->cpumask && !def->cputune.vcpupin &&
 !def->cputune.emulatorpin && !def->cputune.iothreadspin)
 def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
@@ -19894,7 +19894,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 if (cputune)
 virBufferAddLit(buf, "\n");

-if (virDomainNumatuneFormatXML(buf, def->numatune) < 0)
+if (virDomainNumatuneFormatXML(buf, def->numa) < 0)
 goto error;

 if (def->resource)
@@ -22322,7 +22322,7 @@ virDomainDefNeedsPlacementAdvice(virDomainDefPtr def)
 if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
 return true;

-if (virDomainNumatuneHasPlacementAuto(def->numatune))
+if (virDomainNumatuneHasPlacementAuto(def->numa))
 return true;

 return false;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c45e303..1e04886 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2091,7 +2091,7 @@ struct _virDomainDef {

 virDomainCputune cputune;

-virDomainNumatunePtr numatune;
+virDomainNumaPtr numa;
 virDomainResourceDefPtr resource;
 virDomainIdMapDef idmap;

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index d8f1739..bcb8023 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -43,10 +43,10 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
   "static",
   "auto");

-typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
-typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+typedef struct _virDomainNumaNode virDomainNumaNode;
+typedef virDomainNumaNode *virDomainNumaNodePtr;

-struct _virDomainNumatune {
+struct _virDomainNuma {
 struct {
 bool specified;
 virBitmapPtr nodeset;
@@ -54,7 +54,7 @@ struct _virDomainNumatune {
 virDomainNumatunePlacement placement;
 } memory;   /* pinning for all the memory */

-struct _virDomainNumatuneNode {
+struct _virDomainNumaNode {
 virBitmapPtr nodeset;
 virDomainNumatuneMemMode mode;
 } *mem_nodes;   /* fine tuning per guest node */
@@ -65,7 +65,7 @@ struct _virDomainNumatune {


 static inline bool
-virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
+virDomainNumatuneNodeSpecified(virDomainNumaPtr numatune,
int cellid)
 {
 if (numatune &&
@@ -77,7 +77,7 @@ virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
 }

 static int
-virDomainNumatuneNodeParseXML(virDomainNumatunePtr *numatunePtr,
+virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr,
   size_t ncells,
   xmlXPathContextPtr ctxt)
 {
@@ -85,7 +85,7 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr 
*numatunePtr,
 int n = 0;
 int ret = -1;
 size_t i = 0;
-virDomainNumatunePtr numatune = *numatunePtr;
+virDomainNumaPtr numatune = *numatunePtr;
 xmlNodePtr *nodes = NULL;

 if ((n = virXPathNodeSet("./numatune/memnode", ctxt, &nodes)) < 0) {
@@ -126,7 +126,7 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr 
*numatunePtr,
 for (i = 0; i < n; i++) {
 int mode = 0;
 unsigned int cellid = 0;
-

[libvirt] [PATCH 21/24] conf: numa: Add accesor for the NUMA node cpu mask

[Peter Krempa]

Add virDomainNumaGetNodeCpumask() and refactor a few places that would
get the cpu mask without the helper.
---
 src/conf/numa_conf.c | 12 ++--
 src/conf/numa_conf.h |  2 ++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  |  2 +-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 61dfea0..acda415 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -771,7 +771,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
 for (i = 0; i < ncells; i++) {
 memAccess = def->cells[i].memAccess;

-if (!(cpustr = virBitmapFormat(def->cells[i].cpumask)))
+if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i
 return -1;

 virBufferAddLit(buf, "ncells; i++)
-ret += virBitmapCountBits(numa->cells[i].cpumask);
+ret += virBitmapCountBits(virDomainNumaGetNodeCpumask(numa, i));

 return ret;
 }
@@ -824,3 +824,11 @@ virDomainNumaGetNodeCount(virCPUDefPtr numa)

 return numa->ncells;
 }
+
+
+virBitmapPtr
+virDomainNumaGetNodeCpumask(virCPUDefPtr numa,
+size_t node)
+{
+return numa->cells[node].cpumask;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 55a9fbe..ab88ab7 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -87,6 +87,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr 
numatune,
  int cellid);

 size_t virDomainNumaGetNodeCount(virCPUDefPtr numa);
+virBitmapPtr virDomainNumaGetNodeCpumask(virCPUDefPtr numa,
+ size_t node);

 /*
  * Formatters
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4ba5fd0..770803b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -630,6 +630,7 @@ virNodeDeviceObjUnlock;
 virDomainNumaEquals;
 virDomainNumaFree;
 virDomainNumaGetNodeCount;
+virDomainNumaGetNodeCpumask;
 virDomainNumaNew;
 virDomainNumatuneFormatNodeset;
 virDomainNumatuneFormatXML;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f009570..d276da3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7196,7 +7196,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,

 for (i = 0; i < ncells; i++) {
 VIR_FREE(cpumask);
-if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
+if (!(cpumask = virBitmapFormat(virDomainNumaGetNodeCpumask(def->cpu, 
i
 goto cleanup;

 if (strchr(cpumask, ',') &&
-- 
2.2.2

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

[libvirt] [PATCH 01/24] conf: Move numatune_conf to numa_conf

[Peter Krempa]

For a while now there are two places that gather information about NUMA
related guest configuration. While the XML can't be changed we can at
least store the data in one place in the definition.

Rename the numatune_conf.[ch] files to numa_conf as later patches will
move the rest of the definitions from the cpu definition to this one.
---
 po/POTFILES.in|  2 +-
 src/Makefile.am   |  2 +-
 src/conf/domain_conf.h|  2 +-
 src/conf/{numatune_conf.c => numa_conf.c} |  6 +++---
 src/conf/{numatune_conf.h => numa_conf.h} | 10 +-
 src/libvirt_private.syms  |  2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)
 rename src/conf/{numatune_conf.c => numa_conf.c} (99%)
 rename src/conf/{numatune_conf.h => numa_conf.h} (96%)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 3064037..c25bfce 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -27,7 +27,7 @@ src/conf/netdev_vport_profile_conf.c
 src/conf/network_conf.c
 src/conf/networkcommon_conf.c
 src/conf/node_device_conf.c
-src/conf/numatune_conf.c
+src/conf/numa_conf.c
 src/conf/nwfilter_conf.c
 src/conf/nwfilter_params.c
 src/conf/object_event.c
diff --git a/src/Makefile.am b/src/Makefile.am
index b41c6d4..85624be 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -273,7 +273,7 @@ DOMAIN_CONF_SOURCES =   
\
conf/domain_audit.c conf/domain_audit.h \
conf/domain_nwfilter.c conf/domain_nwfilter.h   \
conf/snapshot_conf.c conf/snapshot_conf.h   \
-   conf/numatune_conf.c conf/numatune_conf.h
+   conf/numa_conf.c conf/numa_conf.h

 OBJECT_EVENT_SOURCES = \
conf/object_event.c conf/object_event.h \
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 325afa8..c45e303 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -38,7 +38,7 @@
 # include "virsocketaddr.h"
 # include "networkcommon_conf.h"
 # include "nwfilter_params.h"
-# include "numatune_conf.h"
+# include "numa_conf.h"
 # include "virnetdevmacvlan.h"
 # include "virsysinfo.h"
 # include "virnetdevvportprofile.h"
diff --git a/src/conf/numatune_conf.c b/src/conf/numa_conf.c
similarity index 99%
rename from src/conf/numatune_conf.c
rename to src/conf/numa_conf.c
index 7f322ea..f6a8248 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numa_conf.c
@@ -1,7 +1,7 @@
 /*
- * numatune_conf.c
+ * numa_conf.c
  *
- * Copyright (C) 2014 Red Hat, Inc.
+ * Copyright (C) 2014-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -22,7 +22,7 @@

 #include 

-#include "numatune_conf.h"
+#include "numa_conf.h"

 #include "domain_conf.h"
 #include "viralloc.h"
diff --git a/src/conf/numatune_conf.h b/src/conf/numa_conf.h
similarity index 96%
rename from src/conf/numatune_conf.h
rename to src/conf/numa_conf.h
index 28c4ce2..bcc8c8a 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numa_conf.h
@@ -1,7 +1,7 @@
 /*
- * numatune_conf.h
+ * numa_conf.h
  *
- * Copyright (C) 2014 Red Hat, Inc.
+ * Copyright (C) 2014-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -20,8 +20,8 @@
  * Author: Martin Kletzander 
  */

-#ifndef __NUMATUNE_CONF_H__
-# define __NUMATUNE_CONF_H__
+#ifndef __NUMA_CONF_H__
+# define __NUMA_CONF_H__

 # include 

@@ -111,4 +111,4 @@ int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr 
numatune);

 bool virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune,
  virBitmapPtr auto_nodeset);
-#endif /* __NUMATUNE_CONF_H__ */
+#endif /* __NUMA_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c07a561..edd54b8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -625,7 +625,7 @@ virNodeDeviceObjRemove;
 virNodeDeviceObjUnlock;


-# conf/numatune_conf.h
+# conf/numa_conf.h
 virDomainNumatuneEquals;
 virDomainNumatuneFormatNodeset;
 virDomainNumatuneFormatXML;
-- 
2.2.2

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

[libvirt] [PATCH 09/24] conf: numa: Improve error message in case a numa node doesn't have cpus

[Peter Krempa]

Currently the code would exit without reporting an error as
virBitmapParse reports one only if it fails to parse the bitmap, whereas
the code was jumping to the error label even in case 0 cpus were
correctly parsed in the map.
---
 src/conf/numa_conf.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 41a7e01..391ef15 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -749,9 +749,14 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 }

 if (virBitmapParse(tmp, 0, &def->cells[cur_cell].cpumask,
-   VIR_DOMAIN_CPUMASK_LEN) <= 0)
+   VIR_DOMAIN_CPUMASK_LEN) < 0)
 goto cleanup;

+if (virBitmapIsAllClear(def->cells[cur_cell].cpumask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+  _("NUMA cell %d has no vCPUs assigned"), cur_cell);
+goto cleanup;
+}
 VIR_FREE(tmp);

 ctxt->node = nodes[i];
-- 
2.2.2

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

[libvirt] [PATCH 02/24] conf: Move NUMA cell parsing code from cpu conf to numa conf

[Peter Krempa]

For weird historical reasons NUMA cells are added as a subelement of
 while the actual configuration is done in .

This patch splits out the cell parser code from cpu config to NUMA
config. Note that the changes to the code are minimal just to make it
work and the function will be refactored in the next patch.
---
 src/conf/cpu_conf.c|  90 ---
 src/conf/domain_conf.c |  17 +---
 src/conf/numa_conf.c   | 111 +
 src/conf/numa_conf.h   |   4 ++
 4 files changed, 126 insertions(+), 96 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 4a367a1..f14b37a 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -426,96 +426,6 @@ virCPUDefParseXML(xmlNodePtr node,
 def->features[i].policy = policy;
 }

-if (virXPathNode("./numa[1]", ctxt)) {
-VIR_FREE(nodes);
-n = virXPathNodeSet("./numa[1]/cell", ctxt, &nodes);
-if (n <= 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("NUMA topology defined without NUMA cells"));
-goto error;
-}
-
-if (VIR_RESIZE_N(def->cells, def->ncells_max,
- def->ncells, n) < 0)
-goto error;
-
-def->ncells = n;
-
-for (i = 0; i < n; i++) {
-char *cpus, *memAccessStr;
-int ret, ncpus = 0;
-unsigned int cur_cell;
-char *tmp = NULL;
-
-tmp = virXMLPropString(nodes[i], "id");
-if (!tmp) {
-cur_cell = i;
-} else {
-ret  = virStrToLong_ui(tmp, NULL, 10, &cur_cell);
-if (ret == -1) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid 'id' attribute in NUMA cell: 
%s"),
-   tmp);
-VIR_FREE(tmp);
-goto error;
-}
-VIR_FREE(tmp);
-}
-
-if (cur_cell >= n) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("Exactly one 'cell' element per guest "
- "NUMA cell allowed, non-contiguous ranges or "
- "ranges not starting from 0 are not 
allowed"));
-goto error;
-}
-
-if (def->cells[cur_cell].cpustr) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Duplicate NUMA cell info for cell id '%u'"),
-   cur_cell);
-goto error;
-}
-
-cpus = virXMLPropString(nodes[i], "cpus");
-if (!cpus) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("Missing 'cpus' attribute in NUMA cell"));
-goto error;
-}
-def->cells[cur_cell].cpustr = cpus;
-
-ncpus = virBitmapParse(cpus, 0, &def->cells[cur_cell].cpumask,
-   VIR_DOMAIN_CPUMASK_LEN);
-if (ncpus <= 0)
-goto error;
-def->cells_cpus += ncpus;
-
-ctxt->node = nodes[i];
-if (virDomainParseMemory("./@memory", "./@unit", ctxt,
- &def->cells[cur_cell].mem, true, false) < 
0)
-goto cleanup;
-
-memAccessStr = virXMLPropString(nodes[i], "memAccess");
-if (memAccessStr) {
-int rc = virMemAccessTypeFromString(memAccessStr);
-
-if (rc <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Invalid 'memAccess' attribute "
- "value '%s'"),
-   memAccessStr);
-VIR_FREE(memAccessStr);
-goto error;
-}
-
-def->cells[cur_cell].memAccess = rc;
-
-VIR_FREE(memAccessStr);
-}
-}
-}
-
  cleanup:
 ctxt->node = oldnode;
 VIR_FREE(fallback);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b13cae8..06ed0fd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13495,12 +13495,17 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }

-if (def->cpu->cells_cpus > def->maxvcpus) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Number of CPUs in  exceeds the"
- "  count"));
-goto error;
-}
+}
+
+if (virDomainNumaDefCPUParseXML(def->cpu, ctxt) < 0)
+goto error;
+
+if (def->cpu &&
+def->cpu->cells_cpus > def->maxvcpus) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Number of CPUs in  exceeds the"
+ "  count")

[libvirt] [PATCH 19/24] qemu: command: Unify retrieval of NUMA cell count in qemuBuildNumaArgStr

[Peter Krempa]

The function uses the cell count in 6 places. Add a temp variable to
hold the count as it will greatly simplify the refactor.
---
 src/qemu/qemu_command.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8b660f0..65d8874 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7118,6 +7118,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 bool needBackend = false;
 int rc;
 int ret = -1;
+size_t ncells = def->cpu->ncells;
 const long system_page_size = virGetSystemPageSizeKB();

 if (virDomainNumatuneHasPerNodeBinding(def->numa) &&
@@ -7150,10 +7151,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 continue;
 }

-if (def->cpu->ncells) {
+if (ncells) {
 /* Fortunately, we allow only guest NUMA nodes to be continuous
  * starting from zero. */
-pos = def->cpu->ncells - 1;
+pos = ncells - 1;
 }

 next_bit = virBitmapNextSetBit(def->mem.hugepages[i].nodemask, pos);
@@ -7165,12 +7166,12 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 }
 }

-if (VIR_ALLOC_N(nodeBackends, def->cpu->ncells) < 0)
+if (VIR_ALLOC_N(nodeBackends, ncells) < 0)
 goto cleanup;

 /* using of -numa memdev= cannot be combined with -numa mem=, thus we
  * need to check which approach to use */
-for (i = 0; i < def->cpu->ncells; i++) {
+for (i = 0; i < ncells; i++) {
 unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
 def->cpu->cells[i].mem = cellmem * 1024;

@@ -7193,7 +7194,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 }
 }

-for (i = 0; i < def->cpu->ncells; i++) {
+for (i = 0; i < ncells; i++) {
 VIR_FREE(cpumask);
 if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
 goto cleanup;
@@ -7232,7 +7233,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 VIR_FREE(cpumask);

 if (nodeBackends) {
-for (i = 0; i < def->cpu->ncells; i++)
+for (i = 0; i < ncells; i++)
 VIR_FREE(nodeBackends[i]);

 VIR_FREE(nodeBackends);
-- 
2.2.2

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

[libvirt] [PATCH 15/24] conf: numa: Always allocate the NUMA config

[Peter Krempa]

Since our formatter now handles well if the config is allocated and not
filled and the parser always frees the definition before parsing we can
safely always-allocate the NUMA config.

This will help in later patches as the parser will be refactored to just
fill the data.
---
 src/conf/domain_conf.c   | 10 +-
 src/conf/numa_conf.c | 11 +++
 src/conf/numa_conf.h |  1 +
 src/libvirt_private.syms |  1 +
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6dea109..83c5fd9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2319,9 +2319,17 @@ virDomainDefNew(void)
 {
 virDomainDefPtr ret;

-ignore_value(VIR_ALLOC(ret));
+if (VIR_ALLOC(ret) < 0)
+return NULL;
+
+if (!(ret->numa = virDomainNumaNew()))
+goto error;

 return ret;
+
+ error:
+virDomainDefFree(ret);
+return NULL;
 }


diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 359bdff..2a5fb3c 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -827,3 +827,14 @@ virDomainNumaGetCPUCountTotal(virCPUDefPtr numa)

 return ret;
 }
+
+
+virDomainNumaPtr
+virDomainNumaNew(void)
+{
+virDomainNumaPtr ret = NULL;
+
+ignore_value(VIR_ALLOC(ret));
+
+return ret;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index fa6b89b..e69489d 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -56,6 +56,7 @@ typedef enum {

 VIR_ENUM_DECL(virNumaMemAccess)

+virDomainNumaPtr virDomainNumaNew(void);
 void virDomainNumaFree(virDomainNumaPtr numa);

 /*
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4ba2142..6a746cf 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -629,6 +629,7 @@ virNodeDeviceObjUnlock;
 # conf/numa_conf.h
 virDomainNumaEquals;
 virDomainNumaFree;
+virDomainNumaNew;
 virDomainNumatuneFormatNodeset;
 virDomainNumatuneFormatXML;
 virDomainNumatuneGetMode;
-- 
2.2.2

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

[libvirt] [PATCH 18/24] conf: numa: Don't pass double pointer to virDomainNumatuneParseXML

[Peter Krempa]

virDomainNumatuneParseXML now doesn't allocate the def->numa object any
longer so we don't need to pass a double pointer.
---
 src/conf/domain_conf.c |  2 +-
 src/conf/numa_conf.c   | 23 +++
 src/conf/numa_conf.h   |  2 +-
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 83c5fd9..ceaf092 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13528,7 +13528,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }

-if (virDomainNumatuneParseXML(&def->numa,
+if (virDomainNumatuneParseXML(def->numa,
   def->placement_mode ==
   VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
   def->cpu ? def->cpu->ncells : 0,
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 1b8232e..8adec6f 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -82,7 +82,7 @@ virDomainNumatuneNodeSpecified(virDomainNumaPtr numatune,
 }

 static int
-virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr,
+virDomainNumatuneNodeParseXML(virDomainNumaPtr numa,
   size_t ncells,
   xmlXPathContextPtr ctxt)
 {
@@ -90,7 +90,6 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr,
 int n = 0;
 int ret = -1;
 size_t i = 0;
-virDomainNumaPtr numatune = *numatunePtr;
 xmlNodePtr *nodes = NULL;

 if ((n = virXPathNodeSet("./numatune/memnode", ctxt, &nodes)) < 0) {
@@ -102,8 +101,8 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr,
 if (!n)
 return 0;

-if (numatune && numatune->memory.specified &&
-numatune->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) {
+if (numa->memory.specified &&
+numa->memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Per-node binding is not compatible with "
  "automatic NUMA placement."));
@@ -117,11 +116,11 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr 
*numatunePtr,
 goto cleanup;
 }

-VIR_FREE(numatune->mem_nodes);
-if (VIR_ALLOC_N(numatune->mem_nodes, ncells) < 0)
+VIR_FREE(numa->mem_nodes);
+if (VIR_ALLOC_N(numa->mem_nodes, ncells) < 0)
 goto cleanup;

-numatune->nmem_nodes = ncells;
+numa->nmem_nodes = ncells;

 for (i = 0; i < n; i++) {
 int mode = 0;
@@ -144,14 +143,14 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr 
*numatunePtr,
 }
 VIR_FREE(tmp);

-if (cellid >= numatune->nmem_nodes) {
+if (cellid >= numa->nmem_nodes) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Argument 'cellid' in memnode element must "
  "correspond to existing guest's NUMA cell"));
 goto cleanup;
 }

-mem_node = &numatune->mem_nodes[cellid];
+mem_node = &numa->mem_nodes[cellid];

 if (mem_node->nodeset) {
 virReportError(VIR_ERR_XML_ERROR,
@@ -194,7 +193,7 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr,
 }

 int
-virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,
+virDomainNumatuneParseXML(virDomainNumaPtr numa,
   bool placement_static,
   size_t ncells,
   xmlXPathContextPtr ctxt)
@@ -245,14 +244,14 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,
 VIR_FREE(tmp);
 }

-if (virDomainNumatuneSet(*numatunePtr,
+if (virDomainNumatuneSet(numa,
  placement_static,
  placement,
  mode,
  nodeset) < 0)
 goto cleanup;

-if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) < 0)
+if (virDomainNumatuneNodeParseXML(numa, ncells, ctxt) < 0)
 goto cleanup;

 ret = 0;
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 790f3bf..69eccf2 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -62,7 +62,7 @@ void virDomainNumaFree(virDomainNumaPtr numa);
 /*
  * XML Parse/Format functions
  */
-int virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,
+int virDomainNumatuneParseXML(virDomainNumaPtr numa,
   bool placement_static,
   size_t ncells,
   xmlXPathContextPtr ctxt)
-- 
2.2.2

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

[libvirt] [PATCH 16/24] conf: numa: Avoid re-allocation of the NUMA conf

[Peter Krempa]

As the numa object is now always present we can avoid the ad-hoc
allocation code.
---
 src/conf/numa_conf.c | 26 ++
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 2a5fb3c..70a38d6 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -117,11 +117,6 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr 
*numatunePtr,
 goto cleanup;
 }

-if (!numatune && VIR_ALLOC(numatune) < 0)
-goto cleanup;
-
-*numatunePtr = numatune;
-
 VIR_FREE(numatune->mem_nodes);
 if (VIR_ALLOC_N(numatune->mem_nodes, ncells) < 0)
 goto cleanup;
@@ -224,11 +219,6 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,

 node = virXPathNode("./numatune/memory[1]", ctxt);

-if (*numatunePtr) {
-virDomainNumaFree(*numatunePtr);
-*numatunePtr = NULL;
-}
-
 if (!placement_static && !node)
 placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;

@@ -454,27 +444,20 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr,
  int mode,
  virBitmapPtr nodeset)
 {
-bool created = false;
 int ret = -1;
-virDomainNumaPtr numatune;
+virDomainNumaPtr numatune = *numatunePtr;

 /* No need to do anything in this case */
 if (mode == -1 && placement == -1 && !nodeset)
 return 0;

-if (!(*numatunePtr)) {
-if (VIR_ALLOC(*numatunePtr) < 0)
-goto cleanup;
-
-created = true;
+if (!numatune->memory.specified) {
 if (mode == -1)
 mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
 if (placement == -1)
 placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT;
 }

-numatune = *numatunePtr;
-
 /* Range checks */
 if (mode != -1 &&
 (mode < 0 || mode >= VIR_DOMAIN_NUMATUNE_MEM_LAST)) {
@@ -534,11 +517,6 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr,
 ret = 0;

  cleanup:
-if (ret < 0 && created) {
-virDomainNumaFree(*numatunePtr);
-*numatunePtr = NULL;
-}
-
 return ret;
 }

-- 
2.2.2

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

[libvirt] [PATCH 17/24] numa: conf: Tweak parameters of virDomainNumatuneSet

[Peter Krempa]

As virDomainNumatuneSet now doesn't allocate the virDomainNuma object
any longer it's not necessary to pass the pointer to a pointer to store
the object as it will not change any longer.

While touching the parameter definitions I've also changed the name of
the parameter to "numa".
---
 src/conf/numa_conf.c   | 28 +---
 src/conf/numa_conf.h   |  2 +-
 src/lxc/lxc_native.c   |  2 +-
 src/qemu/qemu_driver.c |  4 ++--
 4 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 70a38d6..1b8232e 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -245,7 +245,7 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,
 VIR_FREE(tmp);
 }

-if (virDomainNumatuneSet(numatunePtr,
+if (virDomainNumatuneSet(*numatunePtr,
  placement_static,
  placement,
  mode,
@@ -438,20 +438,19 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumaPtr 
numatune,
 }

 int
-virDomainNumatuneSet(virDomainNumaPtr *numatunePtr,
+virDomainNumatuneSet(virDomainNumaPtr numa,
  bool placement_static,
  int placement,
  int mode,
  virBitmapPtr nodeset)
 {
 int ret = -1;
-virDomainNumaPtr numatune = *numatunePtr;

 /* No need to do anything in this case */
 if (mode == -1 && placement == -1 && !nodeset)
 return 0;

-if (!numatune->memory.specified) {
+if (!numa->memory.specified) {
 if (mode == -1)
 mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
 if (placement == -1)
@@ -476,26 +475,25 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr,
 }

 if (mode != -1)
-numatune->memory.mode = mode;
+numa->memory.mode = mode;

 if (nodeset) {
-virBitmapFree(numatune->memory.nodeset);
-numatune->memory.nodeset = virBitmapNewCopy(nodeset);
-if (!numatune->memory.nodeset)
+virBitmapFree(numa->memory.nodeset);
+if (!(numa->memory.nodeset = virBitmapNewCopy(nodeset)))
 goto cleanup;
 if (placement == -1)
 placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
 }

 if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT) {
-if (numatune->memory.nodeset || placement_static)
+if (numa->memory.nodeset || placement_static)
 placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
 else
 placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;
 }

 if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC &&
-!numatune->memory.nodeset) {
+!numa->memory.nodeset) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("nodeset for NUMA memory tuning must be set "
  "if 'placement' is 'static'"));
@@ -504,15 +502,15 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr,

 /* setting nodeset when placement auto is invalid */
 if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO &&
-numatune->memory.nodeset) {
-virBitmapFree(numatune->memory.nodeset);
-numatune->memory.nodeset = NULL;
+numa->memory.nodeset) {
+virBitmapFree(numa->memory.nodeset);
+numa->memory.nodeset = NULL;
 }

 if (placement != -1)
-numatune->memory.placement = placement;
+numa->memory.placement = placement;

-numatune->memory.specified = true;
+numa->memory.specified = true;

 ret = 0;

diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index e69489d..790f3bf 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -101,7 +101,7 @@ int virDomainNumatuneMaybeFormatNodeset(virDomainNumaPtr 
numatune,
 /*
  * Setters
  */
-int virDomainNumatuneSet(virDomainNumaPtr *numatunePtr,
+int virDomainNumatuneSet(virDomainNumaPtr numa,
  bool placement_static,
  int placement,
  int mode,
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 99613af..abf07ce 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -847,7 +847,7 @@ lxcSetCpusetTune(virDomainDefPtr def, virConfPtr properties)
 value->str) {
 if (virBitmapParse(value->str, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 
0)
 return -1;
-if (virDomainNumatuneSet(&def->numa,
+if (virDomainNumatuneSet(def->numa,
  def->placement_mode ==
  VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
  VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1c2ace9..0706303 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9543,7 +9543,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 qemuDomainSetNumaParamsLive(vm, nodeset) < 0)
 goto e

[libvirt] [PATCH 20/24] conf: numa: Add helper to get guest NUMA node count and refactor users

[Peter Krempa]

Add an accessor so that a later refactor is simpler.
---
 src/conf/domain_conf.c   |  2 +-
 src/conf/numa_conf.c | 15 +--
 src/conf/numa_conf.h |  2 ++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  |  6 +++---
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ceaf092..d34b9c4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13531,7 +13531,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 if (virDomainNumatuneParseXML(def->numa,
   def->placement_mode ==
   VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
-  def->cpu ? def->cpu->ncells : 0,
+  virDomainNumaGetNodeCount(def->cpu),
   ctxt) < 0)
 goto error;

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 8adec6f..61dfea0 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -760,14 +760,15 @@ virDomainNumaDefCPUFormat(virBufferPtr buf,
 {
 virNumaMemAccess memAccess;
 char *cpustr;
+size_t ncells = virDomainNumaGetNodeCount(def);
 size_t i;

-if (def->ncells == 0)
+if (ncells == 0)
 return 0;

 virBufferAddLit(buf, "\n");
 virBufferAdjustIndent(buf, 2);
-for (i = 0; i < def->ncells; i++) {
+for (i = 0; i < ncells; i++) {
 memAccess = def->cells[i].memAccess;

 if (!(cpustr = virBitmapFormat(def->cells[i].cpumask)))
@@ -813,3 +814,13 @@ virDomainNumaNew(void)

 return ret;
 }
+
+
+size_t
+virDomainNumaGetNodeCount(virCPUDefPtr numa)
+{
+if (!numa)
+return 0;
+
+return numa->ncells;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 69eccf2..55a9fbe 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -86,6 +86,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr 
numatune,
  virBitmapPtr *retNodeset,
  int cellid);

+size_t virDomainNumaGetNodeCount(virCPUDefPtr numa);
+
 /*
  * Formatters
  */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6a746cf..4ba5fd0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -629,6 +629,7 @@ virNodeDeviceObjUnlock;
 # conf/numa_conf.h
 virDomainNumaEquals;
 virDomainNumaFree;
+virDomainNumaGetNodeCount;
 virDomainNumaNew;
 virDomainNumatuneFormatNodeset;
 virDomainNumatuneFormatXML;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 65d8874..f009570 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7118,7 +7118,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 bool needBackend = false;
 int rc;
 int ret = -1;
-size_t ncells = def->cpu->ncells;
+size_t ncells = virDomainNumaGetNodeCount(def->cpu);
 const long system_page_size = virGetSystemPageSizeKB();

 if (virDomainNumatuneHasPerNodeBinding(def->numa) &&
@@ -8315,7 +8315,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, "-m");
 def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024;
 virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024);
-if (def->mem.nhugepages && (!def->cpu || !def->cpu->ncells)) {
+if (def->mem.nhugepages && !virDomainNumaGetNodeCount(def->cpu)) {
 const long system_page_size = virGetSystemPageSizeKB();
 char *mem_path = NULL;

@@ -8395,7 +8395,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 }
 }

-if (def->cpu && def->cpu->ncells)
+if (virDomainNumaGetNodeCount(def->cpu))
 if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
 goto error;

-- 
2.2.2

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

[libvirt] [PATCH 00/24] Move all NUMA related configuration into one structure

[Peter Krempa]

Due to historical madne^Wreasons the NUMA configuration is split between
/domain/cpu and /domain/numatune. Internally the data was also split into two
places. We can't do anything about the external representation but we certainly
can store all the definitions in one place internally.

This series does that.

Peter Krempa (24):
  conf: Move numatune_conf to numa_conf
  conf: Move NUMA cell parsing code from cpu conf to numa conf
  conf: Refactor virDomainNumaDefCPUParseXML
  conf: numa: Don't duplicate NUMA cell cpumask
  conf: Move NUMA cell formatter to numa_conf
  conf: numa: Rename virDomainNumatune to virDomainNuma
  conf: Move enum virMemAccess to the NUMA code and rename it
  conf: numa: Recalculate rather than remember total NUMA cpu count
  conf: numa: Improve error message in case a numa node doesn't have cpus
  conf: numa: Reformat virDomainNumatuneParseXML
  conf: numa: Refactor logic in virDomainNumatuneParseXML
  conf: numa: Format  XML only if necessary
  conf: Separate helper for creating domain objects
  conf: Allocate domain definition with the new helper
  conf: numa: Always allocate the NUMA config
  conf: numa: Avoid re-allocation of the NUMA conf
  numa: conf: Tweak parameters of virDomainNumatuneSet
  conf: numa: Don't pass double pointer to virDomainNumatuneParseXML
  qemu: command: Unify retrieval of NUMA cell count in qemuBuildNumaArgStr
  conf: numa: Add helper to get guest NUMA node count and refactor users
  conf: numa: Add accesor for the NUMA node cpu mask
  conf: numa: Add accessor to NUMA node's memory access mode
  conf: numa: Add setter/getter for NUMA node memory size
  conf: Move all NUMA configuration to virDomainNuma

 po/POTFILES.in |   2 +-
 src/Makefile.am|   2 +-
 src/conf/cpu_conf.c| 151 +---
 src/conf/cpu_conf.h|  25 +-
 src/conf/domain_conf.c |  59 ++-
 src/conf/domain_conf.h |  11 +-
 src/conf/{numatune_conf.c => numa_conf.c}  | 429 +++--
 src/conf/{numatune_conf.h => numa_conf.h}  |  77 ++--
 src/cpu/cpu.c  |   2 +-
 src/libvirt_private.syms   |  13 +-
 src/lxc/lxc_cgroup.c   |   4 +-
 src/lxc/lxc_controller.c   |   6 +-
 src/lxc/lxc_native.c   |   4 +-
 src/openvz/openvz_conf.c   |   2 +-
 src/parallels/parallels_sdk.c  |   4 +-
 src/phyp/phyp_driver.c |   2 +-
 src/qemu/qemu_cgroup.c |  12 +-
 src/qemu/qemu_command.c|  52 +--
 src/qemu/qemu_driver.c |  20 +-
 src/qemu/qemu_process.c|   4 +-
 src/vbox/vbox_common.c |   8 +-
 src/vmx/vmx.c  |   2 +-
 src/xen/xen_hypervisor.c   |   8 +-
 src/xen/xend_internal.c|   4 +-
 src/xen/xm_internal.c  |   4 +-
 src/xenconfig/xen_sxpr.c   |   2 +-
 src/xenconfig/xen_xl.c |   2 +-
 src/xenconfig/xen_xm.c |   2 +-
 tests/cputest.c|   2 +-
 tests/openvzutilstest.c|   2 +-
 .../qemuxml2argv-numatune-memnode.xml  |   2 +-
 .../qemuxml2xmlout-numatune-memnode.xml|   2 +-
 tests/securityselinuxtest.c|   2 +-
 tests/testutilsqemu.c  |   4 -
 34 files changed, 503 insertions(+), 424 deletions(-)
 rename src/conf/{numatune_conf.c => numa_conf.c} (60%)
 rename src/conf/{numatune_conf.h => numa_conf.h} (50%)

-- 
2.2.2

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

[libvirt] [PATCH 14/24] conf: Allocate domain definition with the new helper

[Peter Krempa]

Use the virDomainDefNew() helper to allocate the definition instead of
doing it via VIR_ALLOC.
---
 src/conf/domain_conf.c| 2 +-
 src/lxc/lxc_native.c  | 2 +-
 src/openvz/openvz_conf.c  | 2 +-
 src/parallels/parallels_sdk.c | 2 +-
 src/phyp/phyp_driver.c| 2 +-
 src/qemu/qemu_command.c   | 2 +-
 src/vbox/vbox_common.c| 8 
 src/vmx/vmx.c | 2 +-
 src/xenconfig/xen_sxpr.c  | 2 +-
 src/xenconfig/xen_xl.c| 2 +-
 src/xenconfig/xen_xm.c| 2 +-
 tests/openvzutilstest.c   | 2 +-
 tests/securityselinuxtest.c   | 2 +-
 13 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 420b713..6dea109 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12910,7 +12910,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 VIR_FREE(schema);
 }

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 return NULL;

 if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index cd3b86b..99613af 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -1002,7 +1002,7 @@ lxcParseConfigString(const char *config)
 if (!(properties = virConfReadMem(config, 0, VIR_CONF_FLAG_LXC_FORMAT)))
 return NULL;

-if (VIR_ALLOC(vmdef) < 0)
+if (!(vmdef = virDomainDefNew()))
 goto error;

 if (virUUIDGenerate(vmdef->uuid) < 0) {
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index f955dda..848e230 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -543,7 +543,7 @@ int openvzLoadDomains(struct openvz_driver *driver)
 }
 *line++ = '\0';

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 goto cleanup;

 def->virtType = VIR_DOMAIN_VIRT_OPENVZ;
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 8c05b32..d5ea00a 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1186,7 +1186,7 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
 virCheckNonNullArgGoto(privconn, error);
 virCheckNonNullArgGoto(sdkdom, error);

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 goto error;

 if (!olddom) {
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index d05f897..d69e29c 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1708,7 +1708,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char 
*xml)
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 char *domain_name = NULL;

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 goto cleanup;

 domain_name = escape_specialcharacters(domain->name);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b6fca9c..8b660f0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -11867,7 +11867,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
 return NULL;
 }

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 goto error;

 /* allocate the cmdlinedef up-front; if it's unused, we'll free it later */
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index deca490..55d3624 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3860,7 +3860,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, 
unsigned int flags)
 if (openSessionForMachine(data, dom->uuid, &iid, &machine, false) < 0)
 goto cleanup;

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 goto cleanup;

 gVBoxAPI.UIMachine.GetAccessible(machine, &accessible);
@@ -4114,7 +4114,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom,
 return ret;

 VBOX_IID_INITIALIZE(&iid);
-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 return ret;

 if (VIR_STRDUP(def->os.type, "hvm") < 0)
@@ -4246,7 +4246,7 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const 
char *xml)
 return ret;

 VBOX_IID_INITIALIZE(&iid);
-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 return ret;

 if (VIR_STRDUP(def->os.type, "hvm") < 0)
@@ -6032,7 +6032,7 @@ static char 
*vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
 if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
 goto cleanup;

-if (VIR_ALLOC(def) < 0 || VIR_ALLOC(def->dom) < 0)
+if (VIR_ALLOC(def) < 0 || !(def->dom = virDomainDefNew()))
 goto cleanup;
 if (VIR_STRDUP(def->name, snapshot->name) < 0)
 goto cleanup;
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 2a794c7..ac2542a 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1298,7 +1298,7 @@ virVMXParseConfig(virVMXContext *ctx,
 }

 /* Allocate domain def */
-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 goto cleanup;

 def->virtType = VIR_

[libvirt] [PATCH 11/24] conf: numa: Refactor logic in virDomainNumatuneParseXML

[Peter Krempa]

Shuffling around the logic will allow to simplify the code quite a bit.
As an additional bonus the change in the logic now reports an error if
automatic placement is selected and individual placement is configured.
---
 src/conf/numa_conf.c | 53 +---
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 18c21d5..d5ba27f 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -229,42 +229,31 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr,
 *numatunePtr = NULL;
 }

-if (!node && placement_static) {
-if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) < 0)
-goto cleanup;
-return 0;
-}
+if (!placement_static && !node)
+placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;

-if (!node) {
-/* We know that placement_mode is "auto" if we're here */
-ret = virDomainNumatuneSet(numatunePtr,
-   placement_static,
-   VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO,
-   -1,
-   NULL);
-goto cleanup;
-}
+if (node) {
+if ((tmp = virXMLPropString(node, "mode")) &&
+(mode = virDomainNumatuneMemModeTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unsupported NUMA memory tuning mode '%s'"), tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);

-if ((tmp = virXMLPropString(node, "mode")) &&
-(mode = virDomainNumatuneMemModeTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Unsupported NUMA memory tuning mode '%s'"), tmp);
-goto cleanup;
-}
-VIR_FREE(tmp);
+if ((tmp = virXMLPropString(node, "placement")) &&
+(placement = virDomainNumatunePlacementTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unsupported NUMA memory placement mode '%s'"), 
tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);

-if ((tmp = virXMLPropString(node, "placement")) &&
-(placement = virDomainNumatunePlacementTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Unsupported NUMA memory placement mode '%s'"), tmp);
-goto cleanup;
+if ((tmp = virXMLPropString(node, "nodeset")) &&
+virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
+goto cleanup;
+VIR_FREE(tmp);
 }
-VIR_FREE(tmp);
-
-if ((tmp = virXMLPropString(node, "nodeset")) &&
-virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
-goto cleanup;
-VIR_FREE(tmp);

 if (virDomainNumatuneSet(numatunePtr,
  placement_static,
-- 
2.2.2

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

[libvirt] [PATCH 12/24] conf: numa: Format XML only if necessary

[Peter Krempa]

Do a content-aware check if formatting of the  element is
necessary. Later on the def->numa structure will be always present so we
cannot decide only on the basis whether it's allocated.
---
 src/conf/numa_conf.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index d5ba27f..359bdff 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -278,11 +278,22 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
 {
 const char *tmp = NULL;
 char *nodeset = NULL;
+bool nodesetSpecified = false;
 size_t i = 0;

 if (!numatune)
 return 0;

+for (i = 0; i < numatune->nmem_nodes; i++) {
+if (numatune->mem_nodes[i].nodeset) {
+nodesetSpecified = true;
+break;
+}
+}
+
+if (!nodesetSpecified && !numatune->memory.specified)
+return 0;
+
 virBufferAddLit(buf, "\n");
 virBufferAdjustIndent(buf, 2);

-- 
2.2.2

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

[libvirt] [PATCH 13/24] conf: Separate helper for creating domain objects

[Peter Krempa]

Move the existing virDomainDefNew to virDomainDefNewFull as it's setting
a few things in the conf and re-introduce virDomainDefNew as a function
without parameters for common use.
---
 src/conf/domain_conf.c   | 20 
 src/conf/domain_conf.h   |  7 ---
 src/libvirt_private.syms |  1 +
 src/xen/xen_hypervisor.c |  8 
 src/xen/xend_internal.c  |  4 ++--
 src/xen/xm_internal.c|  4 ++--
 6 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 870efc9..420b713 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2314,13 +2314,25 @@ virDomainObjNew(virDomainXMLOptionPtr xmlopt)
 }


-virDomainDefPtr virDomainDefNew(const char *name,
-const unsigned char *uuid,
-int id)
+virDomainDefPtr
+virDomainDefNew(void)
+{
+virDomainDefPtr ret;
+
+ignore_value(VIR_ALLOC(ret));
+
+return ret;
+}
+
+
+virDomainDefPtr
+virDomainDefNewFull(const char *name,
+const unsigned char *uuid,
+int id)
 {
 virDomainDefPtr def;

-if (VIR_ALLOC(def) < 0)
+if (!(def = virDomainDefNew()))
 return NULL;

 if (VIR_STRDUP(def->name, name) < 0) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1e04886..86db2ab 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2414,9 +2414,10 @@ void virDomainDefFree(virDomainDefPtr vm);

 virDomainChrDefPtr virDomainChrDefNew(void);

-virDomainDefPtr virDomainDefNew(const char *name,
-const unsigned char *uuid,
-int id);
+virDomainDefPtr virDomainDefNew(void);
+virDomainDefPtr virDomainDefNewFull(const char *name,
+const unsigned char *uuid,
+int id);

 enum {
 VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 << 0),
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 278124c..4ba2142 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -204,6 +204,7 @@ virDomainDefMaybeAddController;
 virDomainDefMaybeAddInput;
 virDomainDefNeedsPlacementAdvice;
 virDomainDefNew;
+virDomainDefNewFull;
 virDomainDefParseFile;
 virDomainDefParseNode;
 virDomainDefParseString;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 31a2a1b..bc498ff 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -2634,9 +2634,9 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id)
 if (!name)
 return NULL;

-ret = virDomainDefNew(name,
-  XEN_GETDOMAININFO_UUID(dominfo),
-  id);
+ret = virDomainDefNewFull(name,
+  XEN_GETDOMAININFO_UUID(dominfo),
+  id);
 VIR_FREE(name);
 return ret;
 }
@@ -2699,7 +2699,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const 
unsigned char *uuid)
 if (!name)
 return NULL;

-ret = virDomainDefNew(name, uuid, id);
+ret = virDomainDefNewFull(name, uuid, id);
 if (ret)
 ret->id = id;
 VIR_FREE(name);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index c2b9098..ab03c1c 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1152,7 +1152,7 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr 
*root)
 if (tmp)
 id = sexpr_int(root, "domain/domid");

-return virDomainDefNew(name, uuid, id);
+return virDomainDefNewFull(name, uuid, id);

  error:
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2117,7 +2117,7 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 if (name == NULL)
 return NULL;

-ret = virDomainDefNew(name, uuid, id);
+ret = virDomainDefNewFull(name, uuid, id);

 VIR_FREE(name);
 return ret;
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index dc6f88a..354ab3f 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -844,7 +844,7 @@ xenXMDomainLookupByName(virConnectPtr conn, const char 
*domname)
 if (!(entry = virHashLookup(priv->configCache, filename)))
 goto cleanup;

-ret = virDomainDefNew(domname, entry->def->uuid, -1);
+ret = virDomainDefNewFull(domname, entry->def->uuid, -1);

  cleanup:
 xenUnifiedUnlock(priv);
@@ -887,7 +887,7 @@ xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, 
(const void *)uuid)))
 goto cleanup;

-ret = virDomainDefNew(entry->def->name, uuid, -1);
+ret = virDomainDefNewFull(entry->def->name, uuid, -1);

  cleanup:
 xenUnifiedUnlock(priv);
-- 
2.2.2

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

Re: [libvirt] [PATCH] qemu: check the validity of a pointer

[Peter Krempa]

On Mon, Feb 16, 2015 at 21:17:17 +0100, Matthias Gatto wrote:
> In the current code if mem_mask is NULL there is a "goto error", but
> we freeing it without knowing if mem_mask is NULL or not, therefor
> I've had a check.
> 
> Signed-off-by: Matthias Gatto 
> ---
>  src/qemu/qemu_cgroup.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index ca255c8..926726c 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -810,7 +810,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
>  goto error;
>  
>   cleanup:
> -VIR_FREE(mem_mask);
> +if (mem_mask)
> +VIR_FREE(mem_mask);

NACK,

VIR_FREE actually checks if the pointer is NULL before freeing it.

The real problem is that 'mem_mask' is not initialized to NULL.

Peter


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

[libvirt] [PATCH] qemu: check the validity of a pointer

[Matthias Gatto]

In the current code if mem_mask is NULL there is a "goto error", but
we freeing it without knowing if mem_mask is NULL or not, therefor
I've had a check.

Signed-off-by: Matthias Gatto 
---
 src/qemu/qemu_cgroup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ca255c8..926726c 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -810,7 +810,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
 goto error;
 
  cleanup:
-VIR_FREE(mem_mask);
+if (mem_mask)
+VIR_FREE(mem_mask);
 virBitmapFree(all_nodes);
 return;
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH v3 2/6] qemuProcessHandleBlockJob: Take status into account

[Jiri Denemark]

On Fri, Feb 13, 2015 at 16:24:29 +0100, Michal Privoznik wrote:
> Upon BLOCK_JOB_COMPLETED event delivery, we check if the job has
> completed (in qemuMonitorJSONHandleBlockJobImpl()). For better image,
> the event looks something like this:
> 
> "timestamp": {"seconds": 1423582694, "microseconds": 372666}, "event":
> "BLOCK_JOB_COMPLETED", "data": {"device": "drive-virtio-disk0", "len":
> 8412790784, "offset": 409993216, "speed": 8796093022207, "type":
> "mirror", "error": "No space left on device"}}
> 
> If "len" does not equal "offset" it's considered an error, and we can
> clearly see "error" field filled in. However, later in the event
> processing this case was handled no differently to case of job being
> aborted via separate API. It's time that we start differentiate these
> two because of the future work.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_process.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f30acbb..9cfc0f3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1103,7 +1103,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon 
> ATTRIBUTE_UNUSED,
>  case VIR_DOMAIN_BLOCK_JOB_CANCELED:
>  virStorageSourceFree(disk->mirror);
>  disk->mirror = NULL;
> -disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
> +disk->mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ?
> +VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : 
> VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
>  disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
>  save = true;
>  break;

ACK

Jirka

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

Re: [libvirt] [PATCH v3 1/6] qemuProcessHandleBlockJob: Set disk->mirrorState more often

[Jiri Denemark]

On Fri, Feb 13, 2015 at 16:24:28 +0100, Michal Privoznik wrote:
> Currently, upon BLOCK_JOB_* event, disk->mirrorState is not updated
> each time. The callback code handling the events checks if a blockjob
> was started via our public APIs prior to setting the mirrorState.
> However, some block jobs may be started internally (e.g. during
> storage migration), in which case we don't bother with setting
> disk->mirror (there's nothing we can set it to anyway), or other
> fields. But it will come handy if we update the mirrorState in these
> cases too. The event wasn't delivered just for fun - we've started the
> job after all.
> 
> So, in this commit, the mirrorState is set to whatever job status
> we've obtained. Of course, there are some actions on some statuses
> that we want to perform. But instead of if {} else if {} else {} ...
> enumeration, let's move to switch().
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_process.c | 35 ---
>  1 file changed, 20 insertions(+), 15 deletions(-)

ACK

Jirka

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

Re: [libvirt] [PATCH 2/2] docs: add page about virtlockd setup

[Eric Blake]

On 02/16/2015 04:59 AM, Daniel P. Berrange wrote:
> Introduce some basic docs describing the virtlockd setup.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  docs/locking-lockd.html.in | 160 
> +
>  docs/locking.html.in   |   2 +-
>  docs/sitemap.html.in   |   4 ++
>  3 files changed, 165 insertions(+), 1 deletion(-)
>  create mode 100644 docs/locking-lockd.html.in

> +
> +  The virtlockd daemon is a single purpose binary which
> +  focus exclusively on the task of acquiring and holding

s/focus/focuses/

> +  locks on behalf of running virtual machines. It is
> +  designed to offer a low overhead, portable locking
> +  scheme can be used out of the box on virtualization
> +  hosts with minimal configuration overheads. It makes
> +  use of the POSIX fcntl advisory locking capability
> +  to hold locks, which is supported by the majority of
> +  commonly used filesystems

s/$/./

> +
> +
> +virtlockd daemon setup
> +
> +
> +  In most OS, the virtlockd daemon itself will not require
> +  any upfront configuration work. It is installed by default
> +  when libvirtd is present, and a systemd socket unit is
> +  registered such that the daemon will be automatically
> +  started when first required. With OS that predate systemd
> +  though, it will be neccessary to start it at boot time,

s/neccessary/necessary/


> +
> +  The default behaviour of the lockd plugin is to acquire locks
> +  directly on the virtual disk images associated with the guest
> +   elements. This ensures it can run out of the box
> +  with not configuration, providing locking for disk images on

s/not/no/

-- 
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 v3 4/6] qemuDomainObjPrivate: Introduce blockJob condition

[Jiri Denemark]

On Fri, Feb 13, 2015 at 16:24:31 +0100, Michal Privoznik wrote:
> So far the condition is not used, but will be. There are some
> operations, where we actively wait for a block job to complete.
> Instead of locking and unlocking the domain object, entering and
> leaving monitor, lets just use a condition and let our monitor
> event handling code wake up when needed.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_domain.c | 4 +++-
>  src/qemu/qemu_domain.h | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 99c46d4..28961d2 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -406,7 +406,8 @@ qemuDomainObjPrivateAlloc(void)
>  goto error;
>  }
>  
> -if (virCondInit(&priv->unplugFinished) < 0)
> +if (virCondInit(&priv->unplugFinished) < 0 ||
> +virCondInit(&priv->blockJob) < 0)
>  goto error;
>  
>  if (!(priv->devs = virChrdevAlloc()))
> @@ -439,6 +440,7 @@ qemuDomainObjPrivateFree(void *data)
>  VIR_FREE(priv->origname);
>  
>  virCondDestroy(&priv->unplugFinished);
> +virCondDestroy(&priv->blockJob);
>  virChrdevFree(priv->devs);
>  
>  /* This should never be non-NULL if we get here, but just in case... */
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index b2c3881..db9ffac 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -186,6 +186,8 @@ struct _qemuDomainObjPrivate {
>  const char *unpluggingDevice; /* alias of the device that is being 
> unplugged */
>  char **qemuDevices; /* NULL-terminated list of devices aliases known to 
> QEMU */
>  
> +virCond blockJob; /* signals that one of disks translated state of a 
> block job */

Wouldn't "signals whenever a block job changes its state" or something
similar be more readable?

> +
>  bool hookRun;  /* true if there was a hook run over this domain */
>  virBitmapPtr autoNodeset;
>  };

Jirka

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

Re: [libvirt] [PATCH 0/3] numatune: Prefer old approach

[Jiri Denemark]

On Thu, Feb 12, 2015 at 18:03:50 +0100, Michal Privoznik wrote:
> Consider the following part of domain XML:
> 
>   
>
>   
>   
> 
>   
> 
>   
> 
> Yes, this have a great potential of breaking things. Especially,
> this will break migration between previous two or three upstream
> releases and current release we are working on, because libvirt
> started domains in more complicated way (even if not needed).
> After these patches, domains will be started in simpler way which
> is incompatible.
> 
> On the other hand, we get backward compatibility with much more
> releases than we are about to break.
> 
> Michal Privoznik (3):
>   numatune_conf: Expose virDomainNumatuneNodeSpecified
>   qemuxml2argvtest: Fake response from numad
>   qemuBuildMemoryBackendStr: Report backend requirement more
> appropriately

ACK, but see the two nits I found in 3/3.

Jirka

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

Re: [libvirt] [PATCH 3/3] qemuBuildMemoryBackendStr: Report backend requirement more appropriately

[Jiri Denemark]

On Thu, Feb 12, 2015 at 18:03:53 +0100, Michal Privoznik wrote:
> So, when building the '-numa' command line, the
> qemuBuildMemoryBackendStr() function does quite a lot of checks to
> chose the best backend, or to check if one is in fact needed. However,
> it returned that backend is needed even for this little fella:
> 
>   
> 
>   
> 
> This can be guaranteed via CGroups entirely, there's no need to use
> memory-backend-ram to let qemu know where to get memory from. Well, as
> long as there's no  element, which explicitly requires the
> backend. Long story short, we wouldn't have to care, as qemu works
> either way. However, the problem is migration (as always). Previously,
> libvirt would have started qemu with:
> 
>   -numa node,memory=X
> 
> in this case and restricted memory placement in CGroups. Today, libvirt
> creates more complicated command line:
> 
>   -object memory-backend-ram,id=ram-node0,size=X
>   -numa node,memdev=ram-node0
> 
> Again, one wouldn't find anything wrong with these two approaches.
> Both work just fine. Unless you try to migrated from the older libvirt

s/migrated/migrate/

> into the newer one. These two approaches are, unfortunately, not
> compatible. My suggestion is, in order to allow users to migrate, lets
> use the older approach for as long as the newer one is not needed.
> 
> Signed-off-by: Michal Privoznik 
...
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml 
> b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
> index 01bbb3d..fe7c465 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
> @@ -1,4 +1,4 @@
> -
> +
>dummy2
>4d92ec27-9ebf-400b-ae91-20c71c647c19
>131072

Looks like a useless change.

Jirka

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

[libvirt] Getting Introduced to the Community

[Gaurav Bansal]

Hi,

My name is Gaurav Bansal and I would like to introduce myself to the
developers of this community. I'm (technically) a junior in the Computer
Science degree program at Birla Institute of Technology and Science Pilani.
I am well versed with Python, C/C++, Java, PHP and bash. I have been using
the qemu tool and the virt-manager for almost 6 months now and I'm
comfortable with it.

I was very much fascinated with the concept of Virtualization and wanted to
explore it more and what could be a better way to start off than KVM.

I was having a look at the GSoC 2015 ideas
 page and the
particular project of Running docker containers using virt-sandbox
.
I
have some basic information about the docker images and have worked using
the libvirt API. I have also tried the proof of concept uploaded by Daniel
for downloading the docker images.

Could you please guide me through where should I look forward that would be
helpful for the project?


- Gaurav Bansal

Github : https://github.com/gauravb7090
IRC : gauravb7090
LinkedIn 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] Document virtlockd

[Michal Privoznik]

On 16.02.2015 12:59, Daniel P. Berrange wrote:
> Just some docs additions wrt virtlockd
> 
> Daniel P. Berrange (2):
>   docs: split out sanlock setup docs
>   docs: add page about virtlockd setup
> 
>  docs/locking-lockd.html.in   | 160 +
>  docs/locking-sanlock.html.in | 247 ++
>  docs/locking.html.in | 277 
> ++-
>  docs/sitemap.html.in |  10 ++
>  4 files changed, 450 insertions(+), 244 deletions(-)
>  create mode 100644 docs/locking-lockd.html.in
>  create mode 100644 docs/locking-sanlock.html.in
> 

ACK to both

Michal

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

Re: [libvirt] [RFC PATCH] Use PAUSED state for domains that are starting up

[Daniel P. Berrange]

On Mon, Feb 16, 2015 at 04:03:50PM +0100, Jiri Denemark wrote:
> On Mon, Feb 16, 2015 at 14:57:17 +, Daniel P. Berrange wrote:
> > On Mon, Feb 16, 2015 at 03:50:41PM +0100, Jiri Denemark wrote:
> > > When libvirt is starting a domain, it reports the state as SHUTOFF until
> > > it's RUNNING. This is not ideal because domain startup may take a long
> > > time (usually because of some configuration issues, firewalls blocking
> > > access to network disks, etc.) and domain lists provided by libvirt look
> > > awkward. One can see weird shutoff domains with IDs in a list of active
> > > domains or even shutoff transient domains. In any case, it looks more
> > > like a bug in libvirt than a normal state a domain goes through.
> > 
> > A shutoff transient domain isn't too bad IMHO, but a shutoff domain
> > with an ID number is definitely not expected.
> > 
> > Could we perhaps address it by ensuring that we always return '-1'
> > for ID if the state is "SHUTOFF", even if def->id has a positive
> > value ?
> 
> But we should somehow make it clear that the domain is actually there,
> somehow, only not completely usable. That is, one may need to actually
> call virsh destroy on such domain to get rid of the leftover process if
> something goes wrong.

Hmm, if something goes wrong due virDomainStart though, we should be
tearing down the QEMU process. IIRC we should even be kill -9'ing QEMU,
so even if QEMU is stuck in an uninterruptable sleep and won't exit,
once the (storage?) problem causing that sleep is resolved QEMU will
exit without further intervention. Similarly calling 'destroy' more
times won't make it any more likely to quit, once it has had a SIGKILL

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] [RFC PATCH] Use PAUSED state for domains that are starting up

[Jiri Denemark]

On Mon, Feb 16, 2015 at 14:57:17 +, Daniel P. Berrange wrote:
> On Mon, Feb 16, 2015 at 03:50:41PM +0100, Jiri Denemark wrote:
> > When libvirt is starting a domain, it reports the state as SHUTOFF until
> > it's RUNNING. This is not ideal because domain startup may take a long
> > time (usually because of some configuration issues, firewalls blocking
> > access to network disks, etc.) and domain lists provided by libvirt look
> > awkward. One can see weird shutoff domains with IDs in a list of active
> > domains or even shutoff transient domains. In any case, it looks more
> > like a bug in libvirt than a normal state a domain goes through.
> 
> A shutoff transient domain isn't too bad IMHO, but a shutoff domain
> with an ID number is definitely not expected.
> 
> Could we perhaps address it by ensuring that we always return '-1'
> for ID if the state is "SHUTOFF", even if def->id has a positive
> value ?

But we should somehow make it clear that the domain is actually there,
somehow, only not completely usable. That is, one may need to actually
call virsh destroy on such domain to get rid of the leftover process if
something goes wrong.

Jirka

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

Re: [libvirt] [PATCH 1/2] virFileFindResource: Add support for looking in source tree

[Jiri Denemark]

On Mon, Feb 16, 2015 at 14:26:56 +, Daniel P. Berrange wrote:
> On Mon, Feb 16, 2015 at 03:11:29PM +0100, Jiri Denemark wrote:
> > Not all file we want to find using virFileFindResource{,Full} are
> > generated when libvirt is built, some of them (such as RNG schemas) are
> > distributed with sources. The current API was not able to find source
> > files if libvirt was built in VPATH.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/Makefile.am |  2 ++
> >  src/driver.c|  1 +
> >  src/locking/lock_driver_lockd.c |  1 +
> >  src/locking/lock_manager.c  |  1 +
> >  src/remote/remote_driver.c  |  1 +
> >  src/util/virfile.c  | 39 
> > ++-
> >  src/util/virfile.h  |  8 +++-
> >  7 files changed, 43 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index b41c6d4..a938d7e 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -20,6 +20,7 @@
> >  abs_builddir = $(shell pwd)
> >  abs_topbuilddir = $(shell cd .. && pwd)
> >  abs_srcdir = $(shell cd $(srcdir) && pwd)
> > +abs_topsrcdir = $(shell cd $(srcdir)/.. && pwd)
> >  
> >  # No libraries with the exception of LIBXML should be listed
> >  # here. List them against the individual XXX_la_CFLAGS targets
> > @@ -32,6 +33,7 @@ INCLUDES =-I../gnulib/lib 
> > \
> > -I$(srcdir)/util\
> > -DIN_LIBVIRT\
> > -Dabs_topbuilddir="\"$(abs_topbuilddir)\""  \
> > +   -Dabs_topsrcdir="\"$(abs_topsrcdir)\""  \
> > $(GETTEXT_CPPFLAGS)
> >  
> >  AM_CFLAGS =$(LIBXML_CFLAGS)\
> > diff --git a/src/driver.c b/src/driver.c
> > index 1be32ef..1271597 100644
> > --- a/src/driver.c
> > +++ b/src/driver.c
> > @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name)
> >  "libvirt_driver_",
> >  ".so",
> >  "src/.libs",
> > +
> > VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
> 
> Instead of adding yet another parameter, why don't we just change the
> calling convention of virFileFindResource so that instead of assuming
> abs_topbuilddir we just make the caller prepend the builddir.
> 
> eg instead of pasing 'src/.libs' we pass
> 
> abs_topbuilddir "/src/.libs"
> 
> letting the compiler concatenate these.  The enum you define is
> basically doing exactly that but with an extra level of indirection
> which doesn't seem to buy us anything

Yeah, I guess, that would work too, although it will either need one
more allocation or making all calls to the non-Full variant of this API
more complex. But we can live with it since virFileFind* APIs are used
at about 11 places...

Jirka

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

Re: [libvirt] [RFC PATCH] Use PAUSED state for domains that are starting up

[Daniel P. Berrange]

On Mon, Feb 16, 2015 at 03:50:41PM +0100, Jiri Denemark wrote:
> When libvirt is starting a domain, it reports the state as SHUTOFF until
> it's RUNNING. This is not ideal because domain startup may take a long
> time (usually because of some configuration issues, firewalls blocking
> access to network disks, etc.) and domain lists provided by libvirt look
> awkward. One can see weird shutoff domains with IDs in a list of active
> domains or even shutoff transient domains. In any case, it looks more
> like a bug in libvirt than a normal state a domain goes through.

A shutoff transient domain isn't too bad IMHO, but a shutoff domain
with an ID number is definitely not expected.

Could we perhaps address it by ensuring that we always return '-1'
for ID if the state is "SHUTOFF", even if def->id has a positive
value ?

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] [RFC PATCH] Use PAUSED state for domains that are starting up

[Jiri Denemark]

When libvirt is starting a domain, it reports the state as SHUTOFF until
it's RUNNING. This is not ideal because domain startup may take a long
time (usually because of some configuration issues, firewalls blocking
access to network disks, etc.) and domain lists provided by libvirt look
awkward. One can see weird shutoff domains with IDs in a list of active
domains or even shutoff transient domains. In any case, it looks more
like a bug in libvirt than a normal state a domain goes through.

I'm not quite sure what the best way to fix this is. In this patch, I
tried to use PAUSED state with STARTING_UP reason. Alternatively, we
could keep using SHUTOFF state and just set STARTING_UP reason instead
of UNKNOWN but it just feels wrong and wouldn't really solve the
confusion when looking at virsh list.

I made the change to qemu driver only in this RFC patch, I will update
all drivers once we agree on the best approach.

Signed-off-by: Jiri Denemark 
---
 include/libvirt/libvirt-domain.h |  1 +
 src/conf/domain_conf.c   |  3 ++-
 src/qemu/qemu_process.c  | 22 ++
 tools/virsh-domain-monitor.c |  3 ++-
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 4dbd7f5..90150f6 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -116,6 +116,7 @@ typedef enum {
 VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */
 VIR_DOMAIN_PAUSED_SNAPSHOT = 9,  /* paused while creating a snapshot */
 VIR_DOMAIN_PAUSED_CRASHED = 10, /* paused due to a guest crash */
+VIR_DOMAIN_PAUSED_STARTING_UP = 11, /* the domain is being started */
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_PAUSED_LAST
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3d63f8..2b7c5bf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -661,7 +661,8 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST,
   "from snapshot",
   "shutdown",
   "snapshot",
-  "panicked")
+  "panicked",
+  "starting up")
 
 VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST,
   "unknown",
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1d4e957..d317b19 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3412,6 +3412,7 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, 
virDomainObjPtr vm)
 virDomainState state;
 virDomainPausedReason reason;
 virDomainState newState = VIR_DOMAIN_NOSTATE;
+int oldReason;
 int newReason;
 bool running;
 char *msg = NULL;
@@ -3425,9 +3426,16 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, 
virDomainObjPtr vm)
 if (ret < 0)
 return -1;
 
-state = virDomainObjGetState(vm, NULL);
+state = virDomainObjGetState(vm, &oldReason);
 
-if (state == VIR_DOMAIN_PAUSED && running) {
+if (running &&
+(state == VIR_DOMAIN_SHUTOFF ||
+ (state == VIR_DOMAIN_PAUSED &&
+  oldReason == VIR_DOMAIN_PAUSED_STARTING_UP))) {
+newState = VIR_DOMAIN_RUNNING;
+newReason = VIR_DOMAIN_RUNNING_BOOTED;
+ignore_value(VIR_STRDUP_QUIET(msg, "finished booting"));
+} else if (state == VIR_DOMAIN_PAUSED && running) {
 newState = VIR_DOMAIN_RUNNING;
 newReason = VIR_DOMAIN_RUNNING_UNPAUSED;
 ignore_value(VIR_STRDUP_QUIET(msg, "was unpaused"));
@@ -3446,10 +3454,6 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, 
virDomainObjPtr vm)
 ignore_value(virAsprintf(&msg, "was paused (%s)",
  virDomainPausedReasonTypeToString(reason)));
 }
-} else if (state == VIR_DOMAIN_SHUTOFF && running) {
-newState = VIR_DOMAIN_RUNNING;
-newReason = VIR_DOMAIN_RUNNING_BOOTED;
-ignore_value(VIR_STRDUP_QUIET(msg, "finished booting"));
 }
 
 if (newState != VIR_DOMAIN_NOSTATE) {
@@ -3817,7 +3821,9 @@ qemuProcessReconnect(void *opaque)
 goto error;
 
 state = virDomainObjGetState(obj, &reason);
-if (state == VIR_DOMAIN_SHUTOFF) {
+if (state == VIR_DOMAIN_SHUTOFF ||
+(state == VIR_DOMAIN_PAUSED &&
+ reason == VIR_DOMAIN_PAUSED_STARTING_UP)) {
 VIR_DEBUG("Domain '%s' wasn't fully started yet, killing it",
   obj->def->name);
 goto error;
@@ -4435,7 +4441,7 @@ int qemuProcessStart(virConnectPtr conn,
 
 vm->def->id = qemuDriverAllocateID(driver);
 qemuDomainSetFakeReboot(driver, vm, false);
-virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN);
+virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
 
 if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-doma

Re: [libvirt] [PATCH 1/2] virFileFindResource: Add support for looking in source tree

[Daniel P. Berrange]

On Mon, Feb 16, 2015 at 03:11:29PM +0100, Jiri Denemark wrote:
> Not all file we want to find using virFileFindResource{,Full} are
> generated when libvirt is built, some of them (such as RNG schemas) are
> distributed with sources. The current API was not able to find source
> files if libvirt was built in VPATH.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/Makefile.am |  2 ++
>  src/driver.c|  1 +
>  src/locking/lock_driver_lockd.c |  1 +
>  src/locking/lock_manager.c  |  1 +
>  src/remote/remote_driver.c  |  1 +
>  src/util/virfile.c  | 39 ++-
>  src/util/virfile.h  |  8 +++-
>  7 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index b41c6d4..a938d7e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -20,6 +20,7 @@
>  abs_builddir = $(shell pwd)
>  abs_topbuilddir = $(shell cd .. && pwd)
>  abs_srcdir = $(shell cd $(srcdir) && pwd)
> +abs_topsrcdir = $(shell cd $(srcdir)/.. && pwd)
>  
>  # No libraries with the exception of LIBXML should be listed
>  # here. List them against the individual XXX_la_CFLAGS targets
> @@ -32,6 +33,7 @@ INCLUDES =  -I../gnulib/lib 
> \
>   -I$(srcdir)/util\
>   -DIN_LIBVIRT\
>   -Dabs_topbuilddir="\"$(abs_topbuilddir)\""  \
> + -Dabs_topsrcdir="\"$(abs_topsrcdir)\""  \
>   $(GETTEXT_CPPFLAGS)
>  
>  AM_CFLAGS =  $(LIBXML_CFLAGS)\
> diff --git a/src/driver.c b/src/driver.c
> index 1be32ef..1271597 100644
> --- a/src/driver.c
> +++ b/src/driver.c
> @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name)
>  "libvirt_driver_",
>  ".so",
>  "src/.libs",
> +
> VIR_FILE_FIND_RESOURCE_VPATH_BUILD,

Instead of adding yet another parameter, why don't we just change the
calling convention of virFileFindResource so that instead of assuming
abs_topbuilddir we just make the caller prepend the builddir.

eg instead of pasing 'src/.libs' we pass

abs_topbuilddir "/src/.libs"

letting the compiler concatenate these.  The enum you define is
basically doing exactly that but with an extra level of indirection
which doesn't seem to buy us anything

> @@ -1640,11 +1647,14 @@ virFileFindResourceFull(const char *filename,
>  const char *prefix,
>  const char *suffix,
>  const char *builddir,
> +virFileFindResourceVPath vpath,
>  const char *installdir,
>  const char *envname)
>  {
>  char *ret = NULL;
>  const char *envval = envname ? virGetEnvBlockSUID(envname) : NULL;
> +const char *base = "";
> +const char *path = "";
>  
>  if (!prefix)
>  prefix = "";
> @@ -1652,16 +1662,25 @@ virFileFindResourceFull(const char *filename,
>  suffix = "";
>  
>  if (envval) {
> -if (virAsprintf(&ret, "%s/%s%s%s", envval, prefix, filename, suffix) 
> < 0)
> -return NULL;
> +base = envval;
>  } else if (useDirOverride) {
> -if (virAsprintf(&ret, "%s/%s/%s%s%s", abs_topbuilddir, builddir, 
> prefix, filename, suffix) < 0)
> -return NULL;

eg, simply change this to be

 if (virAsprintf(&ret, "%s/%s/%s%s%s", builddir, prefix, filename, suffix) 
< 0)
   return NULL;


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 2/2] Search for schemas and cpu_map.xml in source tree

[Jiri Denemark]

Both RNG schemas and cpu_map.xml are distributed in source tarball.

Signed-off-by: Jiri Denemark 
---
 src/conf/domain_conf.c   |  6 +++---
 src/cpu/cpu_map.c|  6 +++---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 10 ++
 src/util/virfile.h   |  4 
 5 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b13cae8..b3d63f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12886,9 +12886,9 @@ virDomainDefParseXML(xmlDocPtr xml,
 bool primaryVideo = false;
 
 if (flags & VIR_DOMAIN_DEF_PARSE_VALIDATE) {
-char *schema = virFileFindResource("domain.rng",
-   "docs/schemas",
-   PKGDATADIR "/schemas");
+char *schema = virFileFindResourceSrc("domain.rng",
+  "docs/schemas",
+  PKGDATADIR "/schemas");
 if (!schema)
 return NULL;
 if (virXMLValidateAgainstSchema(schema, xml) < 0) {
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index b77f688..8c005d4 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -86,9 +86,9 @@ int cpuMapLoad(const char *arch,
 int element;
 char *mapfile;
 
-if (!(mapfile = virFileFindResource("cpu_map.xml",
-"src/cpu",
-PKGDATADIR)))
+if (!(mapfile = virFileFindResourceSrc("cpu_map.xml",
+   "src/cpu",
+   PKGDATADIR)))
 return -1;
 
 VIR_DEBUG("Loading CPU map from %s", mapfile);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c07a561..488cde6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1346,6 +1346,7 @@ virFileFindHugeTLBFS;
 virFileFindMountPoint;
 virFileFindResource;
 virFileFindResourceFull;
+virFileFindResourceSrc;
 virFileGetHugepageSize;
 virFileGetMountReverseSubtree;
 virFileGetMountSubtree;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index e60896f..fc3d0a2 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1695,6 +1695,16 @@ virFileFindResource(const char *filename,
installdir, NULL);
 }
 
+char *
+virFileFindResourceSrc(const char *filename,
+   const char *srcdir,
+   const char *installdir)
+{
+return virFileFindResourceFull(filename, NULL, NULL,
+   srcdir, VIR_FILE_FIND_RESOURCE_VPATH_SOURCE,
+   installdir, NULL);
+}
+
 
 /**
  * virFileActivateDirOverride:
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 0e481c2..ddce797 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -172,6 +172,10 @@ char *virFileFindResource(const char *filename,
   const char *builddir,
   const char *installdir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+char *virFileFindResourceSrc(const char *filename,
+ const char *srcdir,
+ const char *installdir)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 char *virFileFindResourceFull(const char *filename,
   const char *prefix,
   const char *suffix,
-- 
2.3.0

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

[libvirt] [PATCH 1/2] virFileFindResource: Add support for looking in source tree

[Jiri Denemark]

Not all file we want to find using virFileFindResource{,Full} are
generated when libvirt is built, some of them (such as RNG schemas) are
distributed with sources. The current API was not able to find source
files if libvirt was built in VPATH.

Signed-off-by: Jiri Denemark 
---
 src/Makefile.am |  2 ++
 src/driver.c|  1 +
 src/locking/lock_driver_lockd.c |  1 +
 src/locking/lock_manager.c  |  1 +
 src/remote/remote_driver.c  |  1 +
 src/util/virfile.c  | 39 ++-
 src/util/virfile.h  |  8 +++-
 7 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b41c6d4..a938d7e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -20,6 +20,7 @@
 abs_builddir = $(shell pwd)
 abs_topbuilddir = $(shell cd .. && pwd)
 abs_srcdir = $(shell cd $(srcdir) && pwd)
+abs_topsrcdir = $(shell cd $(srcdir)/.. && pwd)
 
 # No libraries with the exception of LIBXML should be listed
 # here. List them against the individual XXX_la_CFLAGS targets
@@ -32,6 +33,7 @@ INCLUDES =-I../gnulib/lib 
\
-I$(srcdir)/util\
-DIN_LIBVIRT\
-Dabs_topbuilddir="\"$(abs_topbuilddir)\""  \
+   -Dabs_topsrcdir="\"$(abs_topsrcdir)\""  \
$(GETTEXT_CPPFLAGS)
 
 AM_CFLAGS =$(LIBXML_CFLAGS)\
diff --git a/src/driver.c b/src/driver.c
index 1be32ef..1271597 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -57,6 +57,7 @@ virDriverLoadModule(const char *name)
 "libvirt_driver_",
 ".so",
 "src/.libs",
+VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
 LIBDIR 
"/libvirt/connection-driver",
 "LIBVIRT_DRIVER_DIR")))
 return NULL;
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 2af3f22..53a7256 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -254,6 +254,7 @@ static virNetClientPtr 
virLockManagerLockDaemonConnectionNew(bool privileged,
 !(daemonPath = virFileFindResourceFull("virtlockd",
NULL, NULL,
"src",
+   
VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
SBINDIR,
"VIRTLOCKD_PATH")))
 goto error;
diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
index ec90d04..554e9e2 100644
--- a/src/locking/lock_manager.c
+++ b/src/locking/lock_manager.c
@@ -143,6 +143,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char 
*name,
 NULL,
 ".so",
 "src/.libs",
+
VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
 LIBDIR "/libvirt/lock-driver",
 
"LIBVIRT_LOCK_MANAGER_PLUGIN_DIR")))
 goto cleanup;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index d4fd658..4830a48 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -888,6 +888,7 @@ doRemoteOpen(virConnectPtr conn,
 !(daemonPath = virFileFindResourceFull("libvirtd",
NULL, NULL,
"daemon",
+   
VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
SBINDIR,
"LIBVIRTD_PATH")))
 goto failed;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 1b004d6..e60896f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1618,7 +1618,8 @@ static bool useDirOverride;
  * @filename: libvirt distributed filename without any path
  * @prefix: optional string to prepend to filename
  * @suffix: optional string to append to filename
- * @builddir: location of the binary in the source tree build tree
+ * @builddir: location of the filename in the build tree
+ * @vpath: switch between source and build directory in VPATH build
  * @installdir: location of the installed binary
  * @envname: environment variable used to override all dirs
  *
@@ -1627,8 +1628,14 @@ static bool useDirOverride;
  * run from the source tree. Otherwise it will return the
  * path in the installed location.
  *
+ * If l

[libvirt] [PATCH 0/2] Fix virFileFindResource in VPATH

[Jiri Denemark]

Jiri Denemark (2):
  virFileFindResource: Add support for looking in source tree
  Search for schemas and cpu_map.xml in source tree

 src/Makefile.am |  2 ++
 src/conf/domain_conf.c  |  6 ++---
 src/cpu/cpu_map.c   |  6 ++---
 src/driver.c|  1 +
 src/libvirt_private.syms|  1 +
 src/locking/lock_driver_lockd.c |  1 +
 src/locking/lock_manager.c  |  1 +
 src/remote/remote_driver.c  |  1 +
 src/util/virfile.c  | 49 +
 src/util/virfile.h  | 12 +-
 10 files changed, 64 insertions(+), 16 deletions(-)

-- 
2.3.0

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

[libvirt] [PATCH 0/2] Document virtlockd

[Daniel P. Berrange]

Just some docs additions wrt virtlockd

Daniel P. Berrange (2):
  docs: split out sanlock setup docs
  docs: add page about virtlockd setup

 docs/locking-lockd.html.in   | 160 +
 docs/locking-sanlock.html.in | 247 ++
 docs/locking.html.in | 277 ++-
 docs/sitemap.html.in |  10 ++
 4 files changed, 450 insertions(+), 244 deletions(-)
 create mode 100644 docs/locking-lockd.html.in
 create mode 100644 docs/locking-sanlock.html.in

-- 
2.1.0

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

[libvirt] [PATCH 1/2] docs: split out sanlock setup docs

[Daniel P. Berrange]

In preparation for adding docs about virtlockd, split out
the sanlock setup docs into a separate page.

Signed-off-by: Daniel P. Berrange 
---
 docs/locking-sanlock.html.in | 247 ++
 docs/locking.html.in | 277 ++-
 docs/sitemap.html.in |   6 +
 3 files changed, 286 insertions(+), 244 deletions(-)
 create mode 100644 docs/locking-sanlock.html.in

diff --git a/docs/locking-sanlock.html.in b/docs/locking-sanlock.html.in
new file mode 100644
index 000..84b1fb3
--- /dev/null
+++ b/docs/locking-sanlock.html.in
@@ -0,0 +1,247 @@
+
+http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd";>
+http://www.w3.org/1999/xhtml";>
+  
+Virtual machine lock manager, sanlock plugin
+
+
+
+
+  This page describes use of the
+  https://fedorahosted.org/sanlock/";>sanlock
+  service as a lock driver
+  plugin for virtual machine disk mutual exclusion.
+
+
+Sanlock daemon setup
+
+
+  On many operating systems, the sanlock plugin
+  is distributed in a sub-package which needs to be installed
+  separately from the main libvirt RPM. On a Fedora/RHEL host
+  this can be done with the yum command
+
+
+
+  $ su - root
+  # yum install libvirt-lock-sanlock
+
+
+
+  The next step is to start the sanlock daemon. For maximum
+  safety sanlock prefers to have a connection to a watchdog
+  daemon. This will cause the entire host to be rebooted in
+  the event that sanlock crashes / terminates abnormally.
+  To start the watchdog daemon on a Fedora/RHEL host
+  the following commands can be run:
+
+
+
+  $ su - root
+  # chkconfig wdmd on
+  # service wdmd start
+
+
+
+  Once the watchdog is running, sanlock can be started
+  as follows
+
+
+
+  # chkconfig sanlock on
+  # service sanlock start
+
+
+
+  Note: if you wish to avoid the use of the
+  watchdog, add the following line to /etc/sysconfig/sanlock
+  before starting it
+
+
+
+  SANLOCKOPTS="-w 0"
+
+
+
+  The sanlock daemon must be started on every single host
+  that will be running virtual machines. So repeat these
+  steps as necessary.
+
+
+libvirt sanlock plugin configuration
+
+
+  Once the sanlock daemon is running, the next step is to
+  configure the libvirt sanlock plugin. There is a separate
+  configuration file for each libvirt driver that is using
+  sanlock. For QEMU, we will edit 
/etc/libvirt/qemu-sanlock.conf
+  There is one mandatory parameter that needs to be set,
+  the host_id. This is a integer between
+  1 and 2000, which must be set to a unique
+  value on each host running virtual machines.
+
+
+
+  $ su - root
+  # augtool -s set /files/etc/libvirt/qemu-sanlock.conf/host_id 1
+
+
+
+  Repeat this on every host, changing 1 to a
+  unique value for the host.
+
+
+libvirt sanlock storage configuration
+
+
+  The sanlock plugin needs to create leases in a directory
+  that is on a filesystem shared between all hosts running
+  virtual machines. Obvious choices for this include NFS
+  or GFS2. The libvirt sanlock plugin expects its lease
+  directory be at /var/lib/libvirt/sanlock
+  so update the host's /etc/fstab to mount
+  a suitable shared/cluster filesystem at that location
+
+
+
+  $ su - root
+  # echo "some.nfs.server:/export/sanlock /var/lib/libvirt/sanlock nfs 
hard,nointr 0 0" >> /etc/fstab
+  # mount /var/lib/libvirt/sanlock
+
+
+
+  If your sanlock daemon happen to run under non-root
+  privileges, you need to tell this to libvirt so it
+  chowns created files correctly. This can be done by
+  setting user and/or group
+  variables in the configuration file. Accepted values
+  range is specified in description to the same
+  variables in /etc/libvirt/qemu.conf. For
+  example:
+
+
+
+  augtool -s set /files/etc/libvirt/qemu-sanlock.conf/user sanlock
+  augtool -s set /files/etc/libvirt/qemu-sanlock.conf/group sanlock
+
+
+
+  But remember, that if this is NFS share, you need a
+  no_root_squash-ed one for chown (and chmod possibly)
+  to succeed.
+
+
+
+  In terms of storage requirements, if the filesystem
+  uses 512 byte sectors, you need to allow for 1MB
+  of storage for each guest disk. So if you have a network
+  with 20 virtualization hosts, each running 50 virtual
+  machines and an average of 2 disks per guest, you will
+  need 20*50*2 == 2000 MB of storage for
+  sanlock.
+
+
+
+
+  On one of the hosts on the network is it wise to setup
+  a cron job which runs the virt-sanlock-cleanup
+  script periodically. This scripts deletes any lease
+  files which are not currently in u

[libvirt] [PATCH 2/2] docs: add page about virtlockd setup

[Daniel P. Berrange]

Introduce some basic docs describing the virtlockd setup.

Signed-off-by: Daniel P. Berrange 
---
 docs/locking-lockd.html.in | 160 +
 docs/locking.html.in   |   2 +-
 docs/sitemap.html.in   |   4 ++
 3 files changed, 165 insertions(+), 1 deletion(-)
 create mode 100644 docs/locking-lockd.html.in

diff --git a/docs/locking-lockd.html.in b/docs/locking-lockd.html.in
new file mode 100644
index 000..2b01ee5
--- /dev/null
+++ b/docs/locking-lockd.html.in
@@ -0,0 +1,160 @@
+
+http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd";>
+http://www.w3.org/1999/xhtml";>
+  
+Virtual machine lock manager, virtlockd plugin
+
+
+
+
+  This page describes use of the virtlockd
+  service as a lock driver
+  plugin for virtual machine disk mutual exclusion.
+
+
+virtlockd background
+
+
+  The virtlockd daemon is a single purpose binary which
+  focus exclusively on the task of acquiring and holding
+  locks on behalf of running virtual machines. It is
+  designed to offer a low overhead, portable locking
+  scheme can be used out of the box on virtualization
+  hosts with minimal configuration overheads. It makes
+  use of the POSIX fcntl advisory locking capability
+  to hold locks, which is supported by the majority of
+  commonly used filesystems
+
+
+virtlockd daemon setup
+
+
+  In most OS, the virtlockd daemon itself will not require
+  any upfront configuration work. It is installed by default
+  when libvirtd is present, and a systemd socket unit is
+  registered such that the daemon will be automatically
+  started when first required. With OS that predate systemd
+  though, it will be neccessary to start it at boot time,
+  prior to libvirtd being started. On RHEL/Fedora distros,
+  this can be achieved as follows
+
+
+
+  # chkconfig virtlockd on
+  # service virtlockd start
+
+
+
+  The above instructions apply to the instance of virtlockd
+  that runs privileged, and is used by the libvirtd daemon
+  that runs privileged. If running libvirtd as an unprivileged
+  user, it will always automatically spawn an instance of
+  the virtlockd daemon unprivileged too. This requires no
+  setup at all.
+
+
+libvirt lockd plugin configuration
+
+
+  Once the virtlockd daemon is running, or setup to autostart,
+  the next step is to configure the libvirt lockd plugin.
+  There is a separate configuration file for each libvirt
+  driver that is using virtlockd. For QEMU, we will edit
+  /etc/libvirt/qemu-lockd.conf
+
+
+
+  The default behaviour of the lockd plugin is to acquire locks
+  directly on the virtual disk images associated with the guest
+   elements. This ensures it can run out of the box
+  with not configuration, providing locking for disk images on
+  shared filesystems such as NFS. It does not provide any cross
+  host protection for storage that is backed by block devices,
+  since locks acquired on device nodes in /dev only apply within
+  the host. It may also be the case that the filesystem holding
+  the disk images is not capable of supporting fcntl locks.
+
+
+
+  To address these problems it is possible to tell lockd to
+  acquire locks on an indirect file. Essentially lockd will
+  calculate the SHA256 checksum of the fully qualified path,
+  and create a zero length file in a given directory whose
+  filename is the checksum. It will then acquire a lock on
+  that file. Assuming the block devices assigned to the guest
+  are using stable paths (eg /dev/disk/by-path/XXX) then
+  this will allow for locks to apply across hosts. This
+  feature can be enabled by setting a configuration setting
+  that specifies the directory in which to create the lock
+  files. The directory referred to should of course be
+  placed on a shared filesystem (eg NFS) that is accessible
+  to all hosts which can see the shared block devices.
+
+
+
+  $ su - root
+  # augtool -s set \
+/files/etc/libvirt/qemu-lockd.conf/file_lockspace_dir \
+"/var/lib/libvirt/lockd/files"
+
+
+
+  If the guests are using either LVM and SCSI block devices
+  for their virtual disks, there is a unique identifier
+  associated with each device. It is possible to tell lockd
+  to use this UUID as the basis for acquiring locks, rather
+  than the SHA256 sum of the filename. The benefit of this
+  is that the locking protection will work even if the file
+  paths to the given block device are different on each
+  host.
+
+
+
+  $ su - root
+  # augtool -s set \
+/files/etc/libvirt/qemu-lockd.conf/scsi_lockspace_dir \
+"/var/lib/libvirt/lockd/scsi"
+  # augtool -s set \
+/fi

Re: [libvirt] [PATCH] SRIOV NIC offload feature discovery

[Daniel P. Berrange]

On Mon, Feb 16, 2015 at 10:18:32AM +, James Chapman wrote:
> Adding functionality to libvirt that will allow it
> query the ethtool interface for the availability
> of certain NIC HW offload features
> ---
>  src/conf/device_conf.h |   6 ++
>  src/conf/node_device_conf.c|   7 ++
>  src/conf/node_device_conf.h|   2 +
>  src/libvirt_private.syms   |   1 +
>  src/node_device/node_device_udev.c |   4 ++
>  src/util/virnetdev.c   | 134 
> +
>  src/util/virnetdev.h   |  12 +++-
>  7 files changed, 165 insertions(+), 1 deletion(-)

If you're modifying the XML parser then you need to also change
the XML schema (docs/schemas/nodedev.rng), add example XML
for testing the schema (tests/nodedevschemadata) and finally
extend the docs (docs/formatnodedev.html.in). Also it is good
to include the proposed XML change in the commit message for
reviewers.

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] virsh: fix IP address in vncdisplay for listen type='network'

[Pavel Hrdina]

On Sun, Feb 15, 2015 at 04:49:09PM +0800, Luyao Huang wrote:
> Just like the fix for domdisplay in commit 1ba815.
> ---
>  tools/virsh-domain.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index dc4a863..2506b89 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -10269,6 +10269,18 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd)
>  
>  listen_addr = virXPathString("string(/domain/devices/graphics"
>   "[@type='vnc']/@listen)", ctxt);
> +if (!listen_addr) {
> +/* The subelement address -  -
> + * *should* have been automatically backfilled into its
> + * parent  (which we just tried to
> + * retrieve into listen_addr above) but in some cases it
> + * isn't, so we also do an explicit check for the
> + * subelement (which, by the way, doesn't exist on libvirt
> + * < 0.9.4, so we really do need to check both places)
> + */
> +listen_addr = virXPathString("string(/domain/devices/graphics"
> + "[@type='vnc']/listen/@address)", ctxt);
> +}
>  if (listen_addr == NULL || STREQ(listen_addr, "0.0.0.0"))
>  vshPrint(ctl, ":%d\n", port-5900);
>  else
> -- 
> 1.8.3.1
> 

ACK and pushed.

Pavel

> --
> 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] Enhancing block/disk migration in libvirt

[Michal Privoznik]

On 16.02.2015 11:16, Daniel P. Berrange wrote:
> On Mon, Feb 16, 2015 at 01:42:13PM +1100, Tony Breeds wrote:
>> Hello all,
>> I'm new to both openstack and libvirt so I may get some of this slightly
>> wrong[1].
>>
>> Here is some context form the openstack world (which at least some of you are
>> aware of).  There are at least 2 open bug against openstack (nova) in the 
>> area
>> of block/disk migration.
>>
>> 1) Live migration fails when the instance has a config-drive[2]
>>Here openstack(nova) fails because a drive that nova expects to be 
>> migrated
>>isn't migrated.
>>
>> 2) libvirt live_snapshot periodically explodes on libvirt 1.2.2 in the 
>> gate[3]
>>Here openstack(nova) fails because a drive that nova expects NOT to be
>>migrated is migrated.
>>
>> To me these are essentially the same bug/issue.  There is no way to 
>> communicate with
>> libvirt the users expectations around block/disk mirgration.
>>
>> My idea so far would be to add an options element to the 'disk' XML node.
>> This element could start with 3 possible states
>>
>> block_migration="default": Let libvirt decide
>> block_migration="yes": This device should be block migrated
>> block_migration="no":  This device should *NOT* be block migrated
>>
>> The absence of this element would be treated as "default" above.
>>
>> This would mean that all existing domain XML would still be valid and have 
>> the
>> expected behaviour and  users (such as opensatck) can be explicit about 
>> deviced
>> that do/do not need to be block migrated.
>>
>> While I'm certainly open to discussing the finer points of the 
>> implementation,
>> right now I'm interested in getting a feel for is this idea generally ok?
> 
> This doesn't really belong in the XML. The XML configs are for controlling
> the guest configuration, not functional behaviour of the APIs. So we need
> to fit any neccessary information into the migration API parameters instead,
> or define a new migration API for it. At minimum we just want a list of
> disks to be migrated.

We have a migrate API that takes an array of virTypedParameter. We can
use that to let users pass a list of disks to migrate.

Michal

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

[libvirt] [PATCH] SRIOV NIC offload feature discovery

[James Chapman]

Adding functionality to libvirt that will allow it
query the ethtool interface for the availability
of certain NIC HW offload features
---
 src/conf/device_conf.h |   6 ++
 src/conf/node_device_conf.c|   7 ++
 src/conf/node_device_conf.h|   2 +
 src/libvirt_private.syms   |   1 +
 src/node_device/node_device_udev.c |   4 ++
 src/util/virnetdev.c   | 134 +
 src/util/virnetdev.h   |  12 +++-
 7 files changed, 165 insertions(+), 1 deletion(-)

diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 7256cdc..091f2f0 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -62,6 +62,12 @@ struct _virInterfaceLink {
 unsigned int speed;  /* link speed in Mbits per second */
 };
 
+typedef struct _virDevFeature virDevFeature;
+typedef virDevFeature *virDevFeaturePtr;
+struct _virDevFeature {
+   char *name; /* device feature */
+};
+
 int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr);
 
 int virDevicePCIAddressParseXML(xmlNodePtr node,
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index a728a00..7f4dbfe 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -437,6 +437,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def)
 virBufferEscapeString(&buf, "%s\n",
   data->net.address);
 virInterfaceLinkFormat(&buf, &data->net.lnk);
+if (data->net.features) {
+for (i = 0; i < data->net.nfeatures; i++) {
+virBufferAsprintf(&buf, "\n",
+data->net.features[i].name);
+}
+}
 if (data->net.subtype != VIR_NODE_DEV_CAP_NET_LAST) {
 const char *subtyp =
 virNodeDevNetCapTypeToString(data->net.subtype);
@@ -1679,6 +1685,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 case VIR_NODE_DEV_CAP_NET:
 VIR_FREE(data->net.ifname);
 VIR_FREE(data->net.address);
+VIR_FREE(data->net.features);
 break;
 case VIR_NODE_DEV_CAP_SCSI_HOST:
 VIR_FREE(data->scsi_host.wwnn);
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index fd5d179..918523a 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -141,6 +141,8 @@ struct _virNodeDevCapsDef {
 char *ifname;
 virInterfaceLink lnk;
 virNodeDevNetCapType subtype;  /* LAST -> no subtype */
+size_t nfeatures;
+virDevFeaturePtr features;
 } net;
 struct {
 unsigned int host;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c07a561..1d165a9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1659,6 +1659,7 @@ virNetDevAddRoute;
 virNetDevClearIPAddress;
 virNetDevDelMulti;
 virNetDevExists;
+virNetDevGetFeatures;
 virNetDevGetIndex;
 virNetDevGetIPv4Address;
 virNetDevGetLinkInfo;
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 03c7a0b..349733f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -719,6 +719,10 @@ static int udevProcessNetworkInterface(struct udev_device 
*device,
 if (virNetDevGetLinkInfo(data->net.ifname, &data->net.lnk) < 0)
 goto out;
 
+if (virNetDevGetFeatures(data->net.ifname, &data->net.features,
+&data->net.nfeatures) < 0)
+goto out;
+
 ret = 0;
 
  out:
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 2a0eb43..e513c85 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2728,3 +2728,137 @@ int virNetDevGetRxFilter(const char *ifname,
 *filter = fil;
 return ret;
 }
+
+#if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ)
+
+/**
+ * _virNetDevFeatureAvailable
+ * This function checks for the availability of a network device feature
+ *
+ * @ifname: name of the interface
+ * @cmd: reference to an ethtool command structure
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+int
+_virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
+{
+int ret = -1;
+int sock = -1;
+virIfreq ifr;
+
+sock = socket(AF_LOCAL, SOCK_DGRAM, 0);
+if (sock < 0) {
+virReportSystemError(errno, "%s", _("Cannot open control socket"));
+goto cleanup;
+}
+
+strcpy(ifr.ifr_name, ifname);
+ifr.ifr_data = (void*) cmd;
+
+if (ioctl(sock, SIOCETHTOOL, &ifr) != 0) {
+/* Privileged command, no error */
+if (errno == EPERM || errno == EINVAL) {
+virReportSystemError(errno, "%s", _("ioctl"));
+/* Some kernels dont support named feature, no error */
+} else if (errno == EOPNOTSUPP) {
+virReportSystemError(errno, "%s", _("Warning"));
+} else {
+virReportSystemError(errno, "

Re: [libvirt] Enhancing block/disk migration in libvirt

[Daniel P. Berrange]

On Mon, Feb 16, 2015 at 01:42:13PM +1100, Tony Breeds wrote:
> Hello all,
> I'm new to both openstack and libvirt so I may get some of this slightly
> wrong[1].
> 
> Here is some context form the openstack world (which at least some of you are
> aware of).  There are at least 2 open bug against openstack (nova) in the area
> of block/disk migration.
> 
> 1) Live migration fails when the instance has a config-drive[2]
>Here openstack(nova) fails because a drive that nova expects to be migrated
>isn't migrated.
> 
> 2) libvirt live_snapshot periodically explodes on libvirt 1.2.2 in the gate[3]
>Here openstack(nova) fails because a drive that nova expects NOT to be
>migrated is migrated.
> 
> To me these are essentially the same bug/issue.  There is no way to 
> communicate with
> libvirt the users expectations around block/disk mirgration.
> 
> My idea so far would be to add an options element to the 'disk' XML node.
> This element could start with 3 possible states
> 
> block_migration="default": Let libvirt decide
> block_migration="yes": This device should be block migrated
> block_migration="no":  This device should *NOT* be block migrated
> 
> The absence of this element would be treated as "default" above.
> 
> This would mean that all existing domain XML would still be valid and have the
> expected behaviour and  users (such as opensatck) can be explicit about 
> deviced
> that do/do not need to be block migrated.
>
> While I'm certainly open to discussing the finer points of the implementation,
> right now I'm interested in getting a feel for is this idea generally ok?

This doesn't really belong in the XML. The XML configs are for controlling
the guest configuration, not functional behaviour of the APIs. So we need
to fit any neccessary information into the migration API parameters instead,
or define a new migration API for it. At minimum we just want a list of
disks to be migrated.

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] SRIOV NIC offload feature discovery

[James Chapman]

Adding functionality to libvirt that will allow it
query the ethtool interface for the availability
of certain NIC HW offload features
---
 src/conf/device_conf.h |   6 ++
 src/conf/node_device_conf.c|   7 ++
 src/conf/node_device_conf.h|   2 +
 src/libvirt_private.syms   |   1 +
 src/node_device/node_device_udev.c |   4 ++
 src/util/virnetdev.c   | 134 +
 src/util/virnetdev.h   |  12 +++-
 7 files changed, 165 insertions(+), 1 deletion(-)

diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 7256cdc..091f2f0 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -62,6 +62,12 @@ struct _virInterfaceLink {
 unsigned int speed;  /* link speed in Mbits per second */
 };
 
+typedef struct _virDevFeature virDevFeature;
+typedef virDevFeature *virDevFeaturePtr;
+struct _virDevFeature {
+   char *name; /* device feature */
+};
+
 int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr);
 
 int virDevicePCIAddressParseXML(xmlNodePtr node,
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index a728a00..7f4dbfe 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -437,6 +437,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def)
 virBufferEscapeString(&buf, "%s\n",
   data->net.address);
 virInterfaceLinkFormat(&buf, &data->net.lnk);
+if (data->net.features) {
+for (i = 0; i < data->net.nfeatures; i++) {
+virBufferAsprintf(&buf, "\n",
+data->net.features[i].name);
+}
+}
 if (data->net.subtype != VIR_NODE_DEV_CAP_NET_LAST) {
 const char *subtyp =
 virNodeDevNetCapTypeToString(data->net.subtype);
@@ -1679,6 +1685,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 case VIR_NODE_DEV_CAP_NET:
 VIR_FREE(data->net.ifname);
 VIR_FREE(data->net.address);
+VIR_FREE(data->net.features);
 break;
 case VIR_NODE_DEV_CAP_SCSI_HOST:
 VIR_FREE(data->scsi_host.wwnn);
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index fd5d179..918523a 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -141,6 +141,8 @@ struct _virNodeDevCapsDef {
 char *ifname;
 virInterfaceLink lnk;
 virNodeDevNetCapType subtype;  /* LAST -> no subtype */
+size_t nfeatures;
+virDevFeaturePtr features;
 } net;
 struct {
 unsigned int host;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c07a561..1d165a9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1659,6 +1659,7 @@ virNetDevAddRoute;
 virNetDevClearIPAddress;
 virNetDevDelMulti;
 virNetDevExists;
+virNetDevGetFeatures;
 virNetDevGetIndex;
 virNetDevGetIPv4Address;
 virNetDevGetLinkInfo;
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 03c7a0b..349733f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -719,6 +719,10 @@ static int udevProcessNetworkInterface(struct udev_device 
*device,
 if (virNetDevGetLinkInfo(data->net.ifname, &data->net.lnk) < 0)
 goto out;
 
+if (virNetDevGetFeatures(data->net.ifname, &data->net.features,
+&data->net.nfeatures) < 0)
+goto out;
+
 ret = 0;
 
  out:
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 2a0eb43..e513c85 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2728,3 +2728,137 @@ int virNetDevGetRxFilter(const char *ifname,
 *filter = fil;
 return ret;
 }
+
+#if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ)
+
+/**
+ * _virNetDevFeatureAvailable
+ * This function checks for the availability of a network device feature
+ *
+ * @ifname: name of the interface
+ * @cmd: reference to an ethtool command structure
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+int
+_virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
+{
+int ret = -1;
+int sock = -1;
+virIfreq ifr;
+
+sock = socket(AF_LOCAL, SOCK_DGRAM, 0);
+if (sock < 0) {
+virReportSystemError(errno, "%s", _("Cannot open control socket"));
+goto cleanup;
+}
+
+strcpy(ifr.ifr_name, ifname);
+ifr.ifr_data = (void*) cmd;
+
+if (ioctl(sock, SIOCETHTOOL, &ifr) != 0) {
+/* Privileged command, no error */
+if (errno == EPERM || errno == EINVAL) {
+virReportSystemError(errno, "%s", _("ioctl"));
+/* Some kernels dont support named feature, no error */
+} else if (errno == EOPNOTSUPP) {
+virReportSystemError(errno, "%s", _("Warning"));
+} else {
+virReportSystemError(errno, "

Re: [libvirt] libxl and non-absolute paths

[Martin Kletzander]

On Fri, Feb 13, 2015 at 03:20:07PM +0100, Stefan Bader wrote:

Just recently we moved to libvirt 1.2.12 for the next release. Which brought up
a few problems when working with configs which we and Debian used to have.

A mild complaint towards the xml validation: it would be really nice of that
would be a bit more specific about what exactly it complains. It took me a while
to realize that "Extra element os in interleave" was trying to tell me
that the string of the loader element within the os section was not an absolute
path.

The issue here is that with libxl, I think the goal was to rather allow the
library to select the path prefix (like for pygrub where the full path got
removed recently). But now the xml validation disagrees.

This would go for bootloader for xenpv and loader (within os) for xenfv. And for
emulator in the device section. Though for that things are a bit more
complicated. The libxl driver now calls that with the help option and decides
from the output whether this is the "traditional" xen forked qemu or the
upstream qemu binary. Then it selects the device model depending on that 
outcome.
Not sure whether the libxl driver could query libxl for the path prefix. Right
now the most straight forward way seems to move back to a full path for the
emulator. At least now, by using the standard qemu binary for everything, we got
a predictable path that does not change with Xen versions. So its possible to
force migrate over to put /usr/bin/qemu-system-i386 there.

But for loader and bootloader, do you think it reasonable to change the
templates from absFilePath to filePath?



Maybe stupid question here... How does the string with the prefix look
like then?  Is it something like pygrub:/path/to/loader ?


-Stefan

--- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng   2015-01-23 12:46:24.
+++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 10:00:43.1616
@@ -258,7 +258,7 @@

  

-
+
  


@@ -1060,7 +1060,7 @@
  

  
-
+

  







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


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

Re: [libvirt] [PATCH] Comment out variables/functions that are unused.

[Martin Kletzander]

On Fri, Feb 13, 2015 at 08:18:53PM +, Gary R Hook wrote:

   Avoids complaints when the compiler is configured to "warn-unused".

   A few files contain unnecessary code that results in the compiler
   erroring out when -Wunused* options are used. Comment out the code
   until such time as it is needed.


---
src/libxl/libxl_conf.c   | 2 ++
tests/virnetsockettest.c | 8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 0555b91..f8db4d2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -305,7 +305,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
regmatch_t subs[4];
char *saveptr = NULL;
size_t i;
+/*
virArch hostarch = caps->host.arch;
+*/



How come you see this unused?  It is used about 100 lines later being
compared to VIR_ARCH_X86_64.


struct guest_arch guest_archs[32];
int nr_guest_archs = 0;
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index 5d91f26..988ab43 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -333,9 +333,10 @@ static int testSocketUNIXAddrs(const void *data 
ATTRIBUTE_UNUSED)
return ret;
}

+/*
static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED)
{
-virNetSocketPtr csock = NULL; /* Client socket */
+virNetSocketPtr csock = NULL; / * Client socket * /


Ugly, guess what happens if someone was about uncomment the function.
If you used #if 0 around the function it'd be way easier, or I might
even consider removing that function, BUT...


char buf[100];
size_t i;
int ret = -1;
@@ -360,10 +361,12 @@ static int testSocketCommandNormal(const void *data 
ATTRIBUTE_UNUSED)
virObjectUnref(csock);
return ret;
}
+*/

+/*
static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED)
{
-virNetSocketPtr csock = NULL; /* Client socket */
+virNetSocketPtr csock = NULL; / * Client socket * /
char buf[100];
int ret = -1;
virCommandPtr cmd = virCommandNewArgList("/bin/cat", "/dev/does-not-exist", 
NULL);
@@ -383,6 +386,7 @@ static int testSocketCommandFail(const void *data 
ATTRIBUTE_UNUSED)
virObjectUnref(csock);
return ret;
}
+*/



.. both of these functions *are* used in the file.  At least
everywhere else than on win32 platforms.  If that's the problem, then
it should be in #ifndef WIN32 the same way as the tests using it are.


struct testSSHData {
const char *nodename;
--
1.9.1

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


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

Re: [libvirt] Enhancing block/disk migration in libvirt

[Kashyap Chamarthy]

On Mon, Feb 16, 2015 at 01:42:13PM +1100, Tony Breeds wrote:
> Hello all,
> I'm new to both openstack and libvirt so I may get some of this slightly
> wrong[1].
> 
> Here is some context form the openstack world (which at least some of you are
> aware of).  There are at least 2 open bug against openstack (nova) in the area
> of block/disk migration.
> 
> 1) Live migration fails when the instance has a config-drive[2]
>Here openstack(nova) fails because a drive that nova expects to be migrated
>isn't migrated.
> 
> 2) libvirt live_snapshot periodically explodes on libvirt 1.2.2 in the gate[3]
>Here openstack(nova) fails because a drive that nova expects NOT to be
>migrated is migrated.

On a related note, some weeks ago, I captured some context and analysis
(including some debugging done by Dan Berrange and Eric Blake on public
IRC channel mid-last year) for the above bug, since the actual bug has
too many comments:


https://kashyapc.fedorapeople.org/virt/openstack/nova-live-snapshot-bug-1334398-context-and-analysis.txt

The cause of it is almost narrowed down here by Dan in this comment

https://bugs.launchpad.net/nova/+bug/1334398/comments/27



-- 
/kashyap

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

Re: [libvirt] libxl and non-absolute paths

[Stefan Bader]

On 16.02.2015 10:18, Martin Kletzander wrote:
> On Fri, Feb 13, 2015 at 03:20:07PM +0100, Stefan Bader wrote:
>> Just recently we moved to libvirt 1.2.12 for the next release. Which brought 
>> up
>> a few problems when working with configs which we and Debian used to have.
>>
>> A mild complaint towards the xml validation: it would be really nice of that
>> would be a bit more specific about what exactly it complains. It took me a 
>> while
>> to realize that "Extra element os in interleave" was trying to tell me
>> that the string of the loader element within the os section was not an 
>> absolute
>> path.
>>
>> The issue here is that with libxl, I think the goal was to rather allow the
>> library to select the path prefix (like for pygrub where the full path got
>> removed recently). But now the xml validation disagrees.
>>
>> This would go for bootloader for xenpv and loader (within os) for xenfv. And 
>> for
>> emulator in the device section. Though for that things are a bit more
>> complicated. The libxl driver now calls that with the help option and decides
>> from the output whether this is the "traditional" xen forked qemu or the
>> upstream qemu binary. Then it selects the device model depending on that 
>> outcome.
>> Not sure whether the libxl driver could query libxl for the path prefix. 
>> Right
>> now the most straight forward way seems to move back to a full path for the
>> emulator. At least now, by using the standard qemu binary for everything, we 
>> got
>> a predictable path that does not change with Xen versions. So its possible to
>> force migrate over to put /usr/bin/qemu-system-i386 there.
>>
>> But for loader and bootloader, do you think it reasonable to change the
>> templates from absFilePath to filePath?
>>
> 
> Maybe stupid question here... How does the string with the prefix look
> like then?  Is it something like pygrub:/path/to/loader ?

No, sorry I should probably have added that: in both cases there is only the
binary name in the config and libxl extends things internally.

So pygrub and hvmloader.

-Stefan

> 
>> -Stefan
>>
>> --- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng   2015-01-23 
>> 12:46:24.
>> +++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 
>> 10:00:43.1616
>> @@ -258,7 +258,7 @@
>> 
>>   
>> 
>> -
>> +
>>   
>> 
>> 
>> @@ -1060,7 +1060,7 @@
>>   
>> 
>>   
>> -
>> +
>> 
>>   
>> 
>>
> 
> 
> 
>> -- 
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list




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