Re: [libvirt] [PATCH 3/4] Resolve valgrind error in remoteConfigGetStringList()

[Laine Stump]

On 06/28/2013 03:25 PM, John Ferlan wrote:
> Commit id 'ed3bac71' introduced the following:
>
> TEST: libvirtdconftest
>    40  OK
> ==25875== 690 (480 direct, 210 indirect) bytes in 30 blocks are definitely 
> lost in loss record 18 of 24
> ==25875==at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
> ==25875==by 0x4C737DF: virAllocN (viralloc.c:152)
> ==25875==by 0x403BC8: remoteConfigGetStringList (libvirtd-config.c:74)
> ==25875==by 0x4042CF: daemonConfigLoadOptions (libvirtd-config.c:382)
> ==25875==by 0x4052F5: daemonConfigLoadData (libvirtd-config.c:479)
> ==25875==by 0x40222C: testCorrupt (libvirtdconftest.c:112)
> ==25875==by 0x40321F: virtTestRun (testutils.c:158)
> ==25875==by 0x401FEE: mymain (libvirtdconftest.c:228)
> ==25875==by 0x40385A: virtTestMain (testutils.c:722)
> ==25875==by 0x37C1021A04: (below main) (libc-start.c:225)
> ==25875==
> PASS: libvirtdconftest
> ---
>  daemon/libvirtd-config.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
> index 6f60256..017d470 100644
> --- a/daemon/libvirtd-config.c
> +++ b/daemon/libvirtd-config.c
> @@ -316,6 +316,12 @@ daemonConfigFree(struct daemonConfig *data)
>  VIR_FREE(data->listen_addr);
>  VIR_FREE(data->tls_port);
>  VIR_FREE(data->tcp_port);
> +tmp = data->access_drivers;
> +while (tmp && *tmp) {
> +VIR_FREE(*tmp);
> +tmp++;
> +}
> +VIR_FREE(data->access_drivers);
>  
>  VIR_FREE(data->unix_sock_ro_perms);
>  VIR_FREE(data->unix_sock_rw_perms);


ACK.

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

Re: [libvirt] [PATCH 3/3] qemu: Set RLIMIT_MEMLOCK when memoryBacking/locked is used

[Laine Stump]

On 06/28/2013 11:04 AM, Jiri Denemark wrote:
> If a domain is configured to have all its memory locked, we need to set
> RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory.
> ---
>  src/qemu/qemu_command.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f902501..278d2f2 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6795,6 +6795,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>  virCommandAddArgFormat(cmd, "mlock=%s",
> def->mem.locked ? "on" : "off");
>  }
> +mlock = def->mem.locked;
>  
>  virCommandAddArg(cmd, "-smp");
>  if (!(smp = qemuBuildSmpArgStr(def, qemuCaps)))

ACK.

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

Re: [libvirt] [PATCH 2/3] qemu: Use qemuDomainMemoryLimit when computing memory for VFIO

[Laine Stump]

On 06/28/2013 11:04 AM, Jiri Denemark wrote:
> ---
>  src/qemu/qemu_command.c | 17 +++--
>  src/qemu/qemu_domain.c  | 16 
>  src/qemu/qemu_hotplug.c | 18 --
>  3 files changed, 31 insertions(+), 20 deletions(-)

ACK, pending a short discussion/explanation of the method used to
computer the memory size in 1/3.

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

Re: [libvirt] [PATCH 1/3] qemu: Move memory limit computation to a reusable function

[Laine Stump]

On 06/28/2013 11:04 AM, Jiri Denemark wrote:
> ---
>  src/qemu/qemu_cgroup.c | 21 ++---
>  src/qemu/qemu_domain.c | 29 +
>  src/qemu/qemu_domain.h |  2 ++
>  3 files changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 5f54ca6..22bf78e 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -461,9 +461,7 @@ static int
>  qemuSetupMemoryCgroup(virDomainObjPtr vm)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> -unsigned long long hard_limit;
>  int rc;
> -int i;
>  
>  if (!virCgroupHasController(priv->cgroup,VIR_CGROUP_CONTROLLER_MEMORY)) {
>  if (vm->def->mem.hard_limit != 0 ||
> @@ -477,23 +475,8 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm)
>  }
>  }
>  
> -hard_limit = vm->def->mem.hard_limit;
> -if (!hard_limit) {
> -/* If there is no hard_limit set, set a reasonable one to avoid
> - * system thrashing caused by exploited qemu.  A 'reasonable
> - * limit' has been chosen:
> - * (1 + k) * (domain memory + total video memory) + (32MB for
> - * cache per each disk) + F
> - * where k = 0.5 and F = 200MB.  The cache for disks is important as
> - * kernel cache on the host side counts into the RSS limit. */
> -hard_limit = vm->def->mem.max_balloon;
> -for (i = 0; i < vm->def->nvideos; i++)
> -hard_limit += vm->def->videos[i]->vram;
> -hard_limit = hard_limit * 1.5 + 204800;
> -hard_limit += vm->def->ndisks * 32768;
> -}
> -
> -rc = virCgroupSetMemoryHardLimit(priv->cgroup, hard_limit);
> +rc = virCgroupSetMemoryHardLimit(priv->cgroup,
> + qemuDomainMemoryLimit(vm->def));
>  if (rc != 0) {
>  virReportSystemError(-rc,
>   _("Unable to set memory hard limit for domain 
> %s"),
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8d79066..77b94ec 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2181,3 +2181,32 @@ cleanup:
>  virObjectUnref(cfg);
>  return ret;
>  }
> +
> +
> +unsigned long long
> +qemuDomainMemoryLimit(virDomainDefPtr def)
> +{
> +unsigned long long mem;
> +int i;
> +
> +if (def->mem.hard_limit) {
> +mem = def->mem.hard_limit;
> +} else {
> +/* If there is no hard_limit set, compute a reasonable one to avoid
> + * system thrashing caused by exploited qemu.  A 'reasonable
> + * limit' has been chosen:
> + * (1 + k) * (domain memory + total video memory) + (32MB for
> + * cache per each disk) + F
> + * where k = 0.5 and F = 200MB.  The cache for disks is important as
> + * kernel cache on the host side counts into the RSS limit.
> + */
> +mem = def->mem.max_balloon;
> +for (i = 0; i < def->nvideos; i++)
> +mem += def->videos[i]->vram;
> +mem *= 1.5;
> +mem += def->ndisks * 32768;
> +mem += 204800;
> +}


I know you're just doing a cut-paste here, but I'm curious how we
arrived at this formula. On systems using vfio, it ends up locking *way*
more memory than previously. For my test guest with 4GB of ram assigned,
the old computation would lock 5GB of ram for the qemu process. With the
new method in this patch it ends up locking just about 7GB. I don't know
if that's a case of the original simple VFIO amount being inadequate, or
the new method goind overboard; just thought there should be at least
minimal discussion before it went in (note that I tried it on a guest
with VFIO passthrough and it does work without problems.

Aside from that, ACK to the idea.

> +
> +return mem;
> +}
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 068a4c3..ade2095 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -351,4 +351,6 @@ extern virDomainXMLPrivateDataCallbacks 
> virQEMUDriverPrivateDataCallbacks;
>  extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace;
>  extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig;
>  
> +unsigned long long qemuDomainMemoryLimit(virDomainDefPtr def);
> +
>  #endif /* __QEMU_DOMAIN_H__ */

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

Re: [libvirt] [PATCH V2] Expose all CPU features in host definition

[Don Dugger]

On Fri, Jun 28, 2013 at 09:03:55PM +0100, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 11:51:32AM -0600, Don Dugger wrote:
> > On Fri, Jun 28, 2013 at 10:24:48AM +0100, Daniel P. Berrange wrote:
> > > On Thu, Jun 27, 2013 at 10:35:58AM -0600, Don Dugger wrote:
> > > > On Mon, Jun 17, 2013 at 10:27:36AM +0200, Jiri Denemark wrote:
> > > > > On Fri, Jun 14, 2013 at 12:32:35 -0600, Don Dugger wrote:
> > > > > > On Fri, Jun 14, 2013 at 03:06:52PM +0200, Jiri Denemark wrote:
> > > > > > > I was just trying to say that it doesn't provide anything more 
> > > > > > > than
> > > > > > > "it's supported by the host CPU", which gives mostly no value in 
> > > > > > > the
> > > > > > > context of libvirt. Can you explain more what the use case is in 
> > > > > > > which a
> > > > > > > virt client would need to know what specific feature are 
> > > > > > > supported by
> > > > > > > host CPU? I feel like we should avoid people from being under the
> > > > > > > impression that they can actually use the CPU capabilities for 
> > > > > > > checking
> > > > > > > whether a host can run guests that require specific CPU features.
> > > > > > 
> > > > > > The specific use case I'm trying to address is a cloud environment 
> > > > > > where,
> > > > > > with hundreds of hosts, you might want to schedule an instance to a 
> > > > > > host
> > > > > > that supports a particular HW acceleration, like AES/NI.  I agree, 
> > > > > > what
> > > > > > I `really` want is an API that shows the capabilities of a specific 
> > > > > > guest
> > > > > > that could be created on the host but, absent that API, at least 
> > > > > > knowing
> > > > > > that a host doesn't support the feature is more information than I 
> > > > > > can get
> > > > > > right now.
> > > > > 
> > > > > Hmm, fair enough. Let's wait a few days for Daniel to return from
> > > > > vacation in case he wants to express his opinion here.
> > > > 
> > > > So, any progress here?
> > > 
> > > I believe the cloud use case above is approaching the problem in the wrong
> > > way. We designed our APIs such that apps should never need to write logic
> > > for comparing CPU features directly. If an application has a set of CPU
> > > features it requires from the host, then it should use a libvirt API to
> > > query whether those features are available, not try to write the CPU
> > > fetaure comparison logic itself.
> > > 
> > > You can already pretty much do this with te virConnectCompareCPU()
> > > method, by passing an XML document which specifies the AES/NI feature
> > > flag that you want to check for support of. Then libvirt will tell
> > > you whether the host CPU can support it. It is entirely possible to
> > > make use of this capability as is in OpenStack.
> > 
> > I don't think this would work with the way scheduling in OpenStack works.
> > The problem is that the OpenStack scheduler doesn't want to query each node
> > in the system on every schedule request (with 100s if not 1,000s of compute
> > nodes this would not be practical).  Instead the scheduler maintains info
> > about all of the compute nodes and, when a request comes in, the scheduler
> > identifies the best compute node for the request and then causes the VM
> > to be started on that node.  Apriori the scheduler doesn't even know which
> > CPU features users are interested in, that information only becomes 
> > available
> > when a schedule request comes in so trying to do a `virConnectCompareCPU()'
> > call at that point in time is too late.
> 
> I think your model for user interaction is wrong here. I don't believe
> that OpenStack should be directly exposing the ability for a user to
> explicitly request individual CPU flags for individual VMs. This is
> too risky from a cloud administrator POV, because it could result in
> a user monopolizing a small subset of machines in the guest with
> particular features.  Instead an administrator should be defining
> new flavours with specific CPU feature characteristics. The user can

That's the exact mechanism that is being proposed, the ability to define
a flavor that specifies capabilities that are required.  The issue is
that the flavor is defined independently from the scheduler, it's only
when a schedule request is made that the flavor is presented to the scheduler
and then the scheduler needs to identify which of 1,000s of nodes can
satisfy that flavor.

There shouldn't be a problem with users monopolizing machines because
the cloud administrator defines the flavors and can charge appropriate
premium prices for specific flavors.

> then choose the flavour with the CPU characteristics. In this way the
> system can know ahead of time what flavours are possible on what
> host, and do you don't need to query all nodes at scheduling time.

Note I am not proposing that we make a query at schedule time.  The
system is already setup to have the compute nodes send configuration
info to the scheduler, all I want to do is sent complete info (e.g. all
of t

Re: [libvirt] [PATCH V2] Expose all CPU features in host definition

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 11:51:32AM -0600, Don Dugger wrote:
> On Fri, Jun 28, 2013 at 10:24:48AM +0100, Daniel P. Berrange wrote:
> > On Thu, Jun 27, 2013 at 10:35:58AM -0600, Don Dugger wrote:
> > > On Mon, Jun 17, 2013 at 10:27:36AM +0200, Jiri Denemark wrote:
> > > > On Fri, Jun 14, 2013 at 12:32:35 -0600, Don Dugger wrote:
> > > > > On Fri, Jun 14, 2013 at 03:06:52PM +0200, Jiri Denemark wrote:
> > > > > > I was just trying to say that it doesn't provide anything more than
> > > > > > "it's supported by the host CPU", which gives mostly no value in the
> > > > > > context of libvirt. Can you explain more what the use case is in 
> > > > > > which a
> > > > > > virt client would need to know what specific feature are supported 
> > > > > > by
> > > > > > host CPU? I feel like we should avoid people from being under the
> > > > > > impression that they can actually use the CPU capabilities for 
> > > > > > checking
> > > > > > whether a host can run guests that require specific CPU features.
> > > > > 
> > > > > The specific use case I'm trying to address is a cloud environment 
> > > > > where,
> > > > > with hundreds of hosts, you might want to schedule an instance to a 
> > > > > host
> > > > > that supports a particular HW acceleration, like AES/NI.  I agree, 
> > > > > what
> > > > > I `really` want is an API that shows the capabilities of a specific 
> > > > > guest
> > > > > that could be created on the host but, absent that API, at least 
> > > > > knowing
> > > > > that a host doesn't support the feature is more information than I 
> > > > > can get
> > > > > right now.
> > > > 
> > > > Hmm, fair enough. Let's wait a few days for Daniel to return from
> > > > vacation in case he wants to express his opinion here.
> > > 
> > > So, any progress here?
> > 
> > I believe the cloud use case above is approaching the problem in the wrong
> > way. We designed our APIs such that apps should never need to write logic
> > for comparing CPU features directly. If an application has a set of CPU
> > features it requires from the host, then it should use a libvirt API to
> > query whether those features are available, not try to write the CPU
> > fetaure comparison logic itself.
> > 
> > You can already pretty much do this with te virConnectCompareCPU()
> > method, by passing an XML document which specifies the AES/NI feature
> > flag that you want to check for support of. Then libvirt will tell
> > you whether the host CPU can support it. It is entirely possible to
> > make use of this capability as is in OpenStack.
> 
> I don't think this would work with the way scheduling in OpenStack works.
> The problem is that the OpenStack scheduler doesn't want to query each node
> in the system on every schedule request (with 100s if not 1,000s of compute
> nodes this would not be practical).  Instead the scheduler maintains info
> about all of the compute nodes and, when a request comes in, the scheduler
> identifies the best compute node for the request and then causes the VM
> to be started on that node.  Apriori the scheduler doesn't even know which
> CPU features users are interested in, that information only becomes available
> when a schedule request comes in so trying to do a `virConnectCompareCPU()'
> call at that point in time is too late.

I think your model for user interaction is wrong here. I don't believe
that OpenStack should be directly exposing the ability for a user to
explicitly request individual CPU flags for individual VMs. This is
too risky from a cloud administrator POV, because it could result in
a user monopolizing a small subset of machines in the guest with
particular features.  Instead an administrator should be defining
new flavours with specific CPU feature characteristics. The user can
then choose the flavour with the CPU characteristics. In this way the
system can know ahead of time what flavours are possible on what
host, and do you don't need to query all nodes at scheduling time.

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] doc: Fix reference to #elementsUSB

[Laine Stump]

On 06/28/2013 01:06 PM, Philipp Hahn wrote:
> aae0fc2a922b3e31dae7648c547fca2ac2587625 removed the #elementsUSB anchor
> but did not update the links to point to the new section #elementsHostDev.

ACK and pushed.
>
> Signed-off-by: Philipp Hahn 
> ---
>  docs/formatdomain.html.in |2 +-
>  docs/formatnode.html.in   |2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d651816..cc4c5ea 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -146,7 +146,7 @@
>  configure in the desired way, which is why per-device boot elements
>  (see disks,
>  network interfaces, and
> -USB and PCI devices sections below) were
> +USB and PCI devices sections below) 
> were
>  introduced and they are the preferred way providing full control over
>  booting order. The boot element and per-device boot
>  elements are mutually exclusive. Since 0.1.3,
> diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
> index ad05a70..5f57a5a 100644
> --- a/docs/formatnode.html.in
> +++ b/docs/formatnode.html.in
> @@ -13,7 +13,7 @@
>prefix virNodeDevice, which deal with management of
>host devices that can be handed to guests via passthrough as
> elements
> -  in the domain XML.
> +  in the domain XML.
>These devices are represented as a hierarchy, where a device on
>a bus has a parent of the bus controller device; the root of the
>hierarchy is the node named "computer".

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

Re: [libvirt] [PATCH 4/4] Resolve valgrind errors for nodedev cap parsing

[Laine Stump]

On 06/28/2013 03:25 PM, John Ferlan wrote:
> There were two errors, one as a direct result of commit id '8807b285'
> and the other from cut-n-paste
>
> TEST: nodedevxml2xmltest
>   ..   14  OK
> ==25735== 3 bytes in 1 blocks are definitely lost in loss record 1 of 24
> ==25735==at 0x4A0887C: malloc (vg_replace_malloc.c:270)
> ==25735==by 0x344D2AF275: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
> ==25735==by 0x4D0C767: virNodeDeviceDefParseNode (node_device_conf.c:997)
> ==25735==by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
> ==25735==by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
> ==25735==by 0x402B2F: virtTestRun (testutils.c:158)
> ==25735==by 0x401B27: mymain (nodedevxml2xmltest.c:81)
> ==25735==by 0x40316A: virtTestMain (testutils.c:722)
> ==25735==by 0x37C1021A04: (below main) (libc-start.c:225)
> ==25735==
> ==25735== 16 bytes in 1 blocks are definitely lost in loss record 10 of 24
> ==25735==at 0x4A08A6E: realloc (vg_replace_malloc.c:662)
> ==25735==by 0x4C7385E: virReallocN (viralloc.c:184)
> ==25735==by 0x4C73906: virExpandN (viralloc.c:214)
> ==25735==by 0x4C73B4A: virInsertElementsN (viralloc.c:324)
> ==25735==by 0x4D0C84C: virNodeDeviceDefParseNode (node_device_conf.c:1026)
> ==25735==by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
> ==25735==by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
> ==25735==by 0x402B2F: virtTestRun (testutils.c:158)
> ==25735==by 0x401B27: mymain (nodedevxml2xmltest.c:81)
> ==25735==by 0x40316A: virtTestMain (testutils.c:722)
> ==25735==by 0x37C1021A04: (below main) (libc-start.c:225)
> ==25735==
> PASS: nodedevxml2xmltest
>
> The first error was resolved by adding a missing VIR_FREE(numberStr); in
> the new function virNodeDevCapPciDevIommuGroupParseXML().
>
> The second error was a bit more opaque as the error was a result of copying
> the free methodolgy of the existing code in virNodeDevCapsDefFree(). The code
> would free each of the entries in the array, but not the memory for the
> array itself.  Added the necessary VIR_FREE(data->pci_dev.iommuGroupDevices)
> and while at it added the missing VIR_FREE(data->pci_dev.virtual_functions)
> although there wasn't a test that tripped across it 

Yeah, I noticed when I added the iommuGroupDevices that neither
virtual_functions nor physical_function are included the in nodedev
parser, nor are they tested in the nodedevxml2xml test. I have that on
my mental list of things to put in after this release.

ACK to this patch too.

I really should run valgrind more often (used to do it all the time, but
I've fallen out of the habit...)


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

Re: [libvirt] [PATCH 2/4] Resolve valgrind error in virStorageBackendCreateQemuImgCmd()

[Laine Stump]

On 06/28/2013 03:25 PM, John Ferlan wrote:
> Commit id '53d5967c' introduced the following:
>
> TEST: storagevolxml2argvtest
>   ..   14  OK
> ==25636== 358 (264 direct, 94 indirect) bytes in 1 blocks are definitely lost 
> in loss record 67 of 75
> ==25636==at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
> ==25636==by 0x4C95791: virAlloc (viralloc.c:124)
> ==25636==by 0x4CA0BB4: virCommandNewArgs (vircommand.c:805)
> ==25636==by 0x4CA0C88: virCommandNew (vircommand.c:789)
> ==25636==by 0x408602: virStorageBackendCreateQemuImgCmd 
> (storage_backend.c:849)
> ==25636==by 0x405427: testCompareXMLToArgvHelper 
> (storagevolxml2argvtest.c:61)
> ==25636==by 0x4064DF: virtTestRun (testutils.c:158)
> ==25636==by 0x40516F: mymain (storagevolxml2argvtest.c:195)
> ==25636==by 0x406B1A: virtTestMain (testutils.c:722)
> ==25636==by 0x37C1021A04: (below main) (libc-start.c:225)
> ==25636==
> PASS: storagevolxml2argvtest
> ---
>  src/storage/storage_backend.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index ae25c89..9a3bcf8 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -865,8 +865,10 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
> do_encryption, preallocate,
> vol->target.format,
> vol->target.compat,
> -   vol->target.features) < 0)
> +   vol->target.features) < 0) {
> +virCommandFree(cmd);
>  return NULL;
> +}
>  if (opts)
>  virCommandAddArgList(cmd, "-o", opts, NULL);
>  VIR_FREE(opts);

ACK.

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

Re: [libvirt] [PATCH 1/4] Resolve valgrind error in virNetDevVlanParse()

[Laine Stump]

On 06/28/2013 03:25 PM, John Ferlan wrote:
> Commit '861d4056' introduced the following:
>
> TEST: networkxml2xmltest
>   ..   18  OK
> ==25504== 7 bytes in 1 blocks are definitely lost in loss record 5 of 23
> ==25504==at 0x4A0887C: malloc (vg_replace_malloc.c:270)
> ==25504==by 0x37C1085D71: strdup (strdup.c:42)
> ==25504==by 0x4CB835F: virStrdup (virstring.c:546)
> ==25504==by 0x4CC5179: virXPathString (virxml.c:90)
> ==25504==by 0x4CC75C2: virNetDevVlanParse (netdev_vlan_conf.c:78)
> ==25504==by 0x4CF928A: virNetworkPortGroupParseXML (network_conf.c:1555)
> ==25504==by 0x4CFE385: virNetworkDefParseXML (network_conf.c:2049)
> ==25504==by 0x4D0113B: virNetworkDefParseNode (network_conf.c:2273)
> ==25504==by 0x4D01254: virNetworkDefParse (network_conf.c:2234)
> ==25504==by 0x401E80: testCompareXMLToXMLHelper (networkxml2xmltest.c:32)
> ==25504==by 0x402D4F: virtTestRun (testutils.c:158)
> ==25504==by 0x401CE9: mymain (networkxml2xmltest.c:110)
> ==25504==
> PASS: networkxml2xmltest
>
> Also changed the label from error to cleanup and adjusted code since it's
> all one exit path
> ---

ACK.

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

[libvirt] [PATCH 0/4] Resolve some recent Valgrind errors

[John Ferlan]

Ran Valgrind today and found, investigated, and resolved the errors seen.

John Ferlan (4):
  Resolve valgrind error in virNetDevVlanParse()
  Resolve valgrind error in virStorageBackendCreateQemuImgCmd()
  Resolve valgrind error in remoteConfigGetStringList()
  Resolve valgrind errors for nodedev cap parsing

 daemon/libvirtd-config.c  |  6 ++
 src/conf/netdev_vlan_conf.c   | 24 +---
 src/conf/node_device_conf.c   |  3 +++
 src/storage/storage_backend.c |  4 +++-
 4 files changed, 25 insertions(+), 12 deletions(-)

-- 
1.8.1.4

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

[libvirt] [PATCH 1/4] Resolve valgrind error in virNetDevVlanParse()

[John Ferlan]

Commit '861d4056' introduced the following:

TEST: networkxml2xmltest
  ..   18  OK
==25504== 7 bytes in 1 blocks are definitely lost in loss record 5 of 23
==25504==at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==25504==by 0x37C1085D71: strdup (strdup.c:42)
==25504==by 0x4CB835F: virStrdup (virstring.c:546)
==25504==by 0x4CC5179: virXPathString (virxml.c:90)
==25504==by 0x4CC75C2: virNetDevVlanParse (netdev_vlan_conf.c:78)
==25504==by 0x4CF928A: virNetworkPortGroupParseXML (network_conf.c:1555)
==25504==by 0x4CFE385: virNetworkDefParseXML (network_conf.c:2049)
==25504==by 0x4D0113B: virNetworkDefParseNode (network_conf.c:2273)
==25504==by 0x4D01254: virNetworkDefParse (network_conf.c:2234)
==25504==by 0x401E80: testCompareXMLToXMLHelper (networkxml2xmltest.c:32)
==25504==by 0x402D4F: virtTestRun (testutils.c:158)
==25504==by 0x401CE9: mymain (networkxml2xmltest.c:110)
==25504==
PASS: networkxml2xmltest

Also changed the label from error to cleanup and adjusted code since it's
all one exit path
---
 src/conf/netdev_vlan_conf.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
index 82ff9e8..880a7ce 100644
--- a/src/conf/netdev_vlan_conf.c
+++ b/src/conf/netdev_vlan_conf.c
@@ -45,18 +45,18 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr 
ctxt, virNetDevVlanPtr de
 
 nTags = virXPathNodeSet("./tag", ctxt, &tagNodes);
 if (nTags < 0)
-goto error;
+goto cleanup;
 
 if (nTags == 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing tag id - each  must have "
  "at least one  subelement"));
-goto error;
+goto cleanup;
 }
 
 if (VIR_ALLOC_N(def->tag, nTags) < 0) {
 virReportOOMError();
-goto error;
+goto cleanup;
 }
 
 def->nativeMode = 0;
@@ -68,18 +68,18 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr 
ctxt, virNetDevVlanPtr de
 if (virXPathULong("string(./@id)", ctxt, &id) < 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing or invalid vlan tag id attribute"));
-goto error;
+goto cleanup;
 }
 if (id > 4095) {
 virReportError(VIR_ERR_XML_ERROR,
_("vlan tag id %lu too large (maximum 4095)"), id);
-goto error;
+goto cleanup;
 }
 if ((nativeMode = virXPathString("string(./@nativeMode)", ctxt))) {
 if (def->nativeMode != 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("duplicate native vlan setting"));
-goto error;
+goto cleanup;
 }
 if ((def->nativeMode
  = virNativeVlanModeTypeFromString(nativeMode)) <= 0) {
@@ -87,8 +87,9 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, 
virNetDevVlanPtr de
_("Invalid \"nativeMode='%s'\" "
  "in vlan  element"),
nativeMode);
-goto error;
+goto cleanup;
 }
+VIR_FREE(nativeMode);
 def->nativeTag = id;
 }
 def->tag[ii] = id;
@@ -110,29 +111,30 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr 
ctxt, virNetDevVlanPtr de
 virReportError(VIR_ERR_XML_ERROR,
_("invalid \"trunk='%s'\" in  - 
trunk='yes' "
  "is required for more than one vlan tag"), 
trunk);
-goto error;
+goto cleanup;
 }
 if (def->nativeMode != 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("invalid configuration in  - 
\"trunk='no'\" is "
  "not allowed with a native vlan id"));
-goto error;
+goto cleanup;
 }
 /* allow (but discard) "trunk='no' if there is a single tag */
 if (STRCASENEQ(trunk, "no")) {
 virReportError(VIR_ERR_XML_ERROR,
_("invalid \"trunk='%s'\" in  "
  "- must be yes or no"), trunk);
-goto error;
+goto cleanup;
 }
 }
 }
 
 ret = 0;
-error:
+cleanup:
 ctxt->node = save;
 VIR_FREE(tagNodes);
 VIR_FREE(trunk);
+VIR_FREE(nativeMode);
 if (ret < 0)
 virNetDevVlanClear(def);
 return ret;
-- 
1.8.1.4

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

[libvirt] [PATCH 4/4] Resolve valgrind errors for nodedev cap parsing

[John Ferlan]

There were two errors, one as a direct result of commit id '8807b285'
and the other from cut-n-paste

TEST: nodedevxml2xmltest
  ..   14  OK
==25735== 3 bytes in 1 blocks are definitely lost in loss record 1 of 24
==25735==at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==25735==by 0x344D2AF275: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
==25735==by 0x4D0C767: virNodeDeviceDefParseNode (node_device_conf.c:997)
==25735==by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
==25735==by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
==25735==by 0x402B2F: virtTestRun (testutils.c:158)
==25735==by 0x401B27: mymain (nodedevxml2xmltest.c:81)
==25735==by 0x40316A: virtTestMain (testutils.c:722)
==25735==by 0x37C1021A04: (below main) (libc-start.c:225)
==25735==
==25735== 16 bytes in 1 blocks are definitely lost in loss record 10 of 24
==25735==at 0x4A08A6E: realloc (vg_replace_malloc.c:662)
==25735==by 0x4C7385E: virReallocN (viralloc.c:184)
==25735==by 0x4C73906: virExpandN (viralloc.c:214)
==25735==by 0x4C73B4A: virInsertElementsN (viralloc.c:324)
==25735==by 0x4D0C84C: virNodeDeviceDefParseNode (node_device_conf.c:1026)
==25735==by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
==25735==by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
==25735==by 0x402B2F: virtTestRun (testutils.c:158)
==25735==by 0x401B27: mymain (nodedevxml2xmltest.c:81)
==25735==by 0x40316A: virtTestMain (testutils.c:722)
==25735==by 0x37C1021A04: (below main) (libc-start.c:225)
==25735==
PASS: nodedevxml2xmltest

The first error was resolved by adding a missing VIR_FREE(numberStr); in
the new function virNodeDevCapPciDevIommuGroupParseXML().

The second error was a bit more opaque as the error was a result of copying
the free methodolgy of the existing code in virNodeDevCapsDefFree(). The code
would free each of the entries in the array, but not the memory for the
array itself.  Added the necessary VIR_FREE(data->pci_dev.iommuGroupDevices)
and while at it added the missing VIR_FREE(data->pci_dev.virtual_functions)
although there wasn't a test that tripped across it (thus it's been lurking
since commit id 'a010165d').
---
 src/conf/node_device_conf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 96742ef..087cebb 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1034,6 +1034,7 @@ virNodeDevCapPciDevIommuGroupParseXML(xmlXPathContextPtr 
ctxt,
 ret = 0;
 cleanup:
 ctxt->node = origNode;
+VIR_FREE(numberStr);
 VIR_FREE(addrNodes);
 VIR_FREE(pciAddr);
 return ret;
@@ -1466,9 +1467,11 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 for (i = 0; i < data->pci_dev.num_virtual_functions; i++) {
 VIR_FREE(data->pci_dev.virtual_functions[i]);
 }
+VIR_FREE(data->pci_dev.virtual_functions);
 for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) {
 VIR_FREE(data->pci_dev.iommuGroupDevices[i]);
 }
+VIR_FREE(data->pci_dev.iommuGroupDevices);
 break;
 case VIR_NODE_DEV_CAP_USB_DEV:
 VIR_FREE(data->usb_dev.product_name);
-- 
1.8.1.4

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

[libvirt] [PATCH 3/4] Resolve valgrind error in remoteConfigGetStringList()

[John Ferlan]

Commit id 'ed3bac71' introduced the following:

TEST: libvirtdconftest
   40  OK
==25875== 690 (480 direct, 210 indirect) bytes in 30 blocks are definitely lost 
in loss record 18 of 24
==25875==at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==25875==by 0x4C737DF: virAllocN (viralloc.c:152)
==25875==by 0x403BC8: remoteConfigGetStringList (libvirtd-config.c:74)
==25875==by 0x4042CF: daemonConfigLoadOptions (libvirtd-config.c:382)
==25875==by 0x4052F5: daemonConfigLoadData (libvirtd-config.c:479)
==25875==by 0x40222C: testCorrupt (libvirtdconftest.c:112)
==25875==by 0x40321F: virtTestRun (testutils.c:158)
==25875==by 0x401FEE: mymain (libvirtdconftest.c:228)
==25875==by 0x40385A: virtTestMain (testutils.c:722)
==25875==by 0x37C1021A04: (below main) (libc-start.c:225)
==25875==
PASS: libvirtdconftest
---
 daemon/libvirtd-config.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 6f60256..017d470 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -316,6 +316,12 @@ daemonConfigFree(struct daemonConfig *data)
 VIR_FREE(data->listen_addr);
 VIR_FREE(data->tls_port);
 VIR_FREE(data->tcp_port);
+tmp = data->access_drivers;
+while (tmp && *tmp) {
+VIR_FREE(*tmp);
+tmp++;
+}
+VIR_FREE(data->access_drivers);
 
 VIR_FREE(data->unix_sock_ro_perms);
 VIR_FREE(data->unix_sock_rw_perms);
-- 
1.8.1.4

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

[libvirt] [PATCH 2/4] Resolve valgrind error in virStorageBackendCreateQemuImgCmd()

[John Ferlan]

Commit id '53d5967c' introduced the following:

TEST: storagevolxml2argvtest
  ..   14  OK
==25636== 358 (264 direct, 94 indirect) bytes in 1 blocks are definitely lost 
in loss record 67 of 75
==25636==at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==25636==by 0x4C95791: virAlloc (viralloc.c:124)
==25636==by 0x4CA0BB4: virCommandNewArgs (vircommand.c:805)
==25636==by 0x4CA0C88: virCommandNew (vircommand.c:789)
==25636==by 0x408602: virStorageBackendCreateQemuImgCmd 
(storage_backend.c:849)
==25636==by 0x405427: testCompareXMLToArgvHelper 
(storagevolxml2argvtest.c:61)
==25636==by 0x4064DF: virtTestRun (testutils.c:158)
==25636==by 0x40516F: mymain (storagevolxml2argvtest.c:195)
==25636==by 0x406B1A: virtTestMain (testutils.c:722)
==25636==by 0x37C1021A04: (below main) (libc-start.c:225)
==25636==
PASS: storagevolxml2argvtest
---
 src/storage/storage_backend.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index ae25c89..9a3bcf8 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -865,8 +865,10 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
do_encryption, preallocate,
vol->target.format,
vol->target.compat,
-   vol->target.features) < 0)
+   vol->target.features) < 0) {
+virCommandFree(cmd);
 return NULL;
+}
 if (opts)
 virCommandAddArgList(cmd, "-o", opts, NULL);
 VIR_FREE(opts);
-- 
1.8.1.4

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

Re: [libvirt] [PATCH V2] Expose all CPU features in host definition

[Don Dugger]

On Fri, Jun 28, 2013 at 10:24:48AM +0100, Daniel P. Berrange wrote:
> On Thu, Jun 27, 2013 at 10:35:58AM -0600, Don Dugger wrote:
> > On Mon, Jun 17, 2013 at 10:27:36AM +0200, Jiri Denemark wrote:
> > > On Fri, Jun 14, 2013 at 12:32:35 -0600, Don Dugger wrote:
> > > > On Fri, Jun 14, 2013 at 03:06:52PM +0200, Jiri Denemark wrote:
> > > > > I was just trying to say that it doesn't provide anything more than
> > > > > "it's supported by the host CPU", which gives mostly no value in the
> > > > > context of libvirt. Can you explain more what the use case is in 
> > > > > which a
> > > > > virt client would need to know what specific feature are supported by
> > > > > host CPU? I feel like we should avoid people from being under the
> > > > > impression that they can actually use the CPU capabilities for 
> > > > > checking
> > > > > whether a host can run guests that require specific CPU features.
> > > > 
> > > > The specific use case I'm trying to address is a cloud environment 
> > > > where,
> > > > with hundreds of hosts, you might want to schedule an instance to a host
> > > > that supports a particular HW acceleration, like AES/NI.  I agree, what
> > > > I `really` want is an API that shows the capabilities of a specific 
> > > > guest
> > > > that could be created on the host but, absent that API, at least knowing
> > > > that a host doesn't support the feature is more information than I can 
> > > > get
> > > > right now.
> > > 
> > > Hmm, fair enough. Let's wait a few days for Daniel to return from
> > > vacation in case he wants to express his opinion here.
> > 
> > So, any progress here?
> 
> I believe the cloud use case above is approaching the problem in the wrong
> way. We designed our APIs such that apps should never need to write logic
> for comparing CPU features directly. If an application has a set of CPU
> features it requires from the host, then it should use a libvirt API to
> query whether those features are available, not try to write the CPU
> fetaure comparison logic itself.
> 
> You can already pretty much do this with te virConnectCompareCPU()
> method, by passing an XML document which specifies the AES/NI feature
> flag that you want to check for support of. Then libvirt will tell
> you whether the host CPU can support it. It is entirely possible to
> make use of this capability as is in OpenStack.

I don't think this would work with the way scheduling in OpenStack works.
The problem is that the OpenStack scheduler doesn't want to query each node
in the system on every schedule request (with 100s if not 1,000s of compute
nodes this would not be practical).  Instead the scheduler maintains info
about all of the compute nodes and, when a request comes in, the scheduler
identifies the best compute node for the request and then causes the VM
to be started on that node.  Apriori the scheduler doesn't even know which
CPU features users are interested in, that information only becomes available
when a schedule request comes in so trying to do a `virConnectCompareCPU()'
call at that point in time is too late.

Having said all of that I still think that conceptually the current
`virConnectGetCapabilities()' is wrong.  If the API is going to list any
CPU features is should list all of them.  Any user of the API that cares
about CPU features probably cares about all of the features and a user that
doesn't care about CPU features will just ignore that part of the definition
anyway.

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

-- 
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
n0...@n0ano.com
Ph: 303/443-3786

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

[libvirt] [PATCH] doc: Fix reference to #elementsUSB

[Philipp Hahn]

aae0fc2a922b3e31dae7648c547fca2ac2587625 removed the #elementsUSB anchor
but did not update the links to point to the new section #elementsHostDev.

Signed-off-by: Philipp Hahn 
---
 docs/formatdomain.html.in |2 +-
 docs/formatnode.html.in   |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d651816..cc4c5ea 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -146,7 +146,7 @@
 configure in the desired way, which is why per-device boot elements
 (see disks,
 network interfaces, and
-USB and PCI devices sections below) were
+USB and PCI devices sections below) were
 introduced and they are the preferred way providing full control over
 booting order. The boot element and per-device boot
 elements are mutually exclusive. Since 0.1.3,
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index ad05a70..5f57a5a 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -13,7 +13,7 @@
   prefix virNodeDevice, which deal with management of
   host devices that can be handed to guests via passthrough as
    elements
-  in the domain XML.
+  in the domain XML.
   These devices are represented as a hierarchy, where a device on
   a bus has a parent of the bus controller device; the root of the
   hierarchy is the node named "computer".
-- 
1.7.10.4

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

Re: [libvirt] [PATCH] Document security reporting & handling process

[Eric Blake]

On 06/04/2013 09:33 AM, Eric Blake wrote:
> On 06/04/2013 04:06 AM, Daniel P. Berrange wrote:
>> From: "Daniel P. Berrange" 
>>
>> Historically security issues in libvirt have been primarily
>> triaged & fixed by the Red Hat libvirt members & Red Hat
>> security team, who then usually notify other vendors via
>> appropriate channels. There have been a number of times
>> when vendors have not been properly notified ahead of
>> announcement. It has also disadvantaged community members
>> who have to backport fixes to releases for which there are
>> no current libvirt stable branches.
>>
>> To address this, we want to make the libvirt security process
>> entirely community focused / driven. To this end I have setup
>> a new email address "libvirt-secur...@redhat.com" for end
>> users to report bugs which have (possible) security implications.

>> Document how to report security bugs and the process that
>> will be used for addressing them.
>>
>> Signed-off-by: Daniel P. Berrange 
>> ---
>>  docs/bugs.html.in|  12 +
>>  docs/contact.html.in |  12 +
>>  docs/securityprocess.html.in | 113 
>> +++
>>  docs/sitemap.html.in |   4 ++
>>  4 files changed, 141 insertions(+)
>>  create mode 100644 docs/securityprocess.html.in

Did this ever get pushed?  It should go in before 1.1.0 is released,
particularly since we have already used this list to discuss
CVE-2013-2218 (more details on Monday when embargo ends).

-- 
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] domain_conf: Include the correct console alias

[Michal Privoznik]

On 28.06.2013 17:01, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 04:45:08PM +0200, Michal Privoznik wrote:
>> On 28.06.2013 15:55, Daniel P. Berrange wrote:
>>> On Fri, Jun 28, 2013 at 03:31:17PM +0200, Michal Privoznik wrote:
 For some crazy backward compatibility, a console can by just an alias to
 a serial device. This is detected in the XML formating function which
 takes the values to format from corresponding serial device. Including
 the device alias. This results in wrong alias being written into the XML
 definition:

 
   ...
   
 

 While holding the correct alias still in the memory, it doesn't matter.
 However, it starts to matter as soon as libvirtd is restarted and the
 (incorrect) alias is read from status file.
> 
> I don't actually see this problem at all.
> 
> Starting with a guest containing
> 
> 
>   
> 
> 
>   
> 
> 
> After virsh start, it contains
> 
> 
>   
>   
>   
> 
> 
>   
>   
>   
> 
> 
> 
> The XML in $XDG_RUNTIME_DIR/libvirt/qemu/run/vm1.xml contains
> exactly the same.
> 
> If i now kill libvirtd and restart it, the XML is still
> reported correctly.
> 
> How do you reproduce the bug ?

The problem is, the internal array (vm->def->consoles) doesn't contain
just 'consoleX' but 'serialX' too after libvirtd restarts. I've noticed
this while testing my chardev hotplug patches. If user wants to hotplug
a console, I need to iterate over vm->def->consoles to find the next
free X to generate the alias. I am using qemuDomainDeviceAliasIndex for
that. The function takes alias prefix as one of its arguments. However,
if the prefix doesn't match, an error is returned. And since the aliases
vary over libvirtd restarts I think that's the problem. Of course, I can
workaround it in my patches, but c'mon we want the libvirt source code
to be hack-less, don't we? :)

> 
>>>
>>> This isn't any more correct that your previous patch. For the
>>> dummy  elements, the alias *must* be copied from /
>>> identical to the corresponding  device config.
>>>
>>>
>>> Daniel
>>>
>>
>> So what are you saying is, it's the  that has the wrong alias
>> assigned? The output should be like this then?
>>
>> 
>>   
>>   
>>   
>>   
>> 
>>
>> 
>>   
>>   
>>   
>> 
> 
> No, the alias name must match that of the device id= parameter in the
> QEMU command line which is 'serial0' for a serial port.
> 
> 
> Daniel
> 

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

[libvirt] [PATCH 1/3] qemu: Move memory limit computation to a reusable function

[Jiri Denemark]

---
 src/qemu/qemu_cgroup.c | 21 ++---
 src/qemu/qemu_domain.c | 29 +
 src/qemu/qemu_domain.h |  2 ++
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 5f54ca6..22bf78e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -461,9 +461,7 @@ static int
 qemuSetupMemoryCgroup(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-unsigned long long hard_limit;
 int rc;
-int i;
 
 if (!virCgroupHasController(priv->cgroup,VIR_CGROUP_CONTROLLER_MEMORY)) {
 if (vm->def->mem.hard_limit != 0 ||
@@ -477,23 +475,8 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm)
 }
 }
 
-hard_limit = vm->def->mem.hard_limit;
-if (!hard_limit) {
-/* If there is no hard_limit set, set a reasonable one to avoid
- * system thrashing caused by exploited qemu.  A 'reasonable
- * limit' has been chosen:
- * (1 + k) * (domain memory + total video memory) + (32MB for
- * cache per each disk) + F
- * where k = 0.5 and F = 200MB.  The cache for disks is important as
- * kernel cache on the host side counts into the RSS limit. */
-hard_limit = vm->def->mem.max_balloon;
-for (i = 0; i < vm->def->nvideos; i++)
-hard_limit += vm->def->videos[i]->vram;
-hard_limit = hard_limit * 1.5 + 204800;
-hard_limit += vm->def->ndisks * 32768;
-}
-
-rc = virCgroupSetMemoryHardLimit(priv->cgroup, hard_limit);
+rc = virCgroupSetMemoryHardLimit(priv->cgroup,
+ qemuDomainMemoryLimit(vm->def));
 if (rc != 0) {
 virReportSystemError(-rc,
  _("Unable to set memory hard limit for domain 
%s"),
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8d79066..77b94ec 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2181,3 +2181,32 @@ cleanup:
 virObjectUnref(cfg);
 return ret;
 }
+
+
+unsigned long long
+qemuDomainMemoryLimit(virDomainDefPtr def)
+{
+unsigned long long mem;
+int i;
+
+if (def->mem.hard_limit) {
+mem = def->mem.hard_limit;
+} else {
+/* If there is no hard_limit set, compute a reasonable one to avoid
+ * system thrashing caused by exploited qemu.  A 'reasonable
+ * limit' has been chosen:
+ * (1 + k) * (domain memory + total video memory) + (32MB for
+ * cache per each disk) + F
+ * where k = 0.5 and F = 200MB.  The cache for disks is important as
+ * kernel cache on the host side counts into the RSS limit.
+ */
+mem = def->mem.max_balloon;
+for (i = 0; i < def->nvideos; i++)
+mem += def->videos[i]->vram;
+mem *= 1.5;
+mem += def->ndisks * 32768;
+mem += 204800;
+}
+
+return mem;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 068a4c3..ade2095 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -351,4 +351,6 @@ extern virDomainXMLPrivateDataCallbacks 
virQEMUDriverPrivateDataCallbacks;
 extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace;
 extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig;
 
+unsigned long long qemuDomainMemoryLimit(virDomainDefPtr def);
+
 #endif /* __QEMU_DOMAIN_H__ */
-- 
1.8.2.1

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

[libvirt] [PATCH 0/3] qemu: Make setting memory limits consistent

[Jiri Denemark]

We need to set memory limits in several cases:
- when setting hard_limit in memory cgroup
- when starting a domain with a VFIO PCI device attached
- when hotplugging a VFIO PCI device
- when memoryBacking/locked is used

This series makes use of a shared code to compute the limits in all of
those cases.

Jiri Denemark (3):
  qemu: Move memory limit computation to a reusable function
  qemu: Use qemuDomainMemoryLimit when computing memory for VFIO
  qemu: Set RLIMIT_MEMLOCK when memoryBacking/locked is used

 src/qemu/qemu_cgroup.c  | 21 ++---
 src/qemu/qemu_command.c | 18 --
 src/qemu/qemu_domain.c  | 45 +
 src/qemu/qemu_domain.h  |  2 ++
 src/qemu/qemu_hotplug.c | 18 --
 5 files changed, 65 insertions(+), 39 deletions(-)

-- 
1.8.2.1

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

[libvirt] [PATCH 2/3] qemu: Use qemuDomainMemoryLimit when computing memory for VFIO

[Jiri Denemark]

---
 src/qemu/qemu_command.c | 17 +++--
 src/qemu/qemu_domain.c  | 16 
 src/qemu/qemu_hotplug.c | 18 --
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4d70004..f902501 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6683,6 +6683,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 int spice = 0;
 int usbcontroller = 0;
 bool usblegacy = false;
+bool mlock = false;
 int contOrder[] = {
 /* We don't add an explicit IDE or FD controller because the
  * provided PIIX4 device already includes one. It isn't possible to
@@ -8337,22 +8338,15 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 if (hostdev->source.subsys.u.pci.backend
 == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-unsigned long long memKB;
-
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("VFIO PCI device assignment is not "
  "supported by this version of qemu"));
 goto error;
 }
-/* VFIO requires all of the guest's memory to be
- * locked resident, plus some amount for IO
- * space. Alex Williamson suggested adding 1GiB for IO
- * space just to be safe (some finer tuning might be
- * nice, though).
- */
-memKB = def->mem.max_balloon + (1024 * 1024);
-virCommandSetMaxMemLock(cmd, memKB * 1024);
+/* VFIO requires all of the guest's memory to be locked
+ * resident */
+mlock = true;
 }
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
@@ -8572,6 +8566,9 @@ qemuBuildCommandLine(virConnectPtr conn,
 goto error;
 }
 
+if (mlock)
+virCommandSetMaxMemLock(cmd, qemuDomainMemoryLimit(def) * 1024);
+
 virObjectUnref(cfg);
 return cmd;
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 77b94ec..8952a79 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2199,6 +2199,10 @@ qemuDomainMemoryLimit(virDomainDefPtr def)
  * cache per each disk) + F
  * where k = 0.5 and F = 200MB.  The cache for disks is important as
  * kernel cache on the host side counts into the RSS limit.
+ *
+ * Moreover, VFIO requires some amount for IO space. Alex Williamson
+ * suggested adding 1GiB for IO space just to be safe (some finer
+ * tuning might be nice, though).
  */
 mem = def->mem.max_balloon;
 for (i = 0; i < def->nvideos; i++)
@@ -2206,6 +2210,18 @@ qemuDomainMemoryLimit(virDomainDefPtr def)
 mem *= 1.5;
 mem += def->ndisks * 32768;
 mem += 204800;
+
+for (i = 0; i < def->nhostdevs; i++) {
+virDomainHostdevDefPtr hostdev = def->hostdevs[i];
+if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+hostdev->source.subsys.type ==
+VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
+hostdev->source.subsys.u.pci.backend ==
+VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
+mem += 1024 * 1024;
+break;
+}
+}
 }
 
 return mem;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 46875ad..a350059 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1054,23 +1054,21 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr 
driver,
 
 if (hostdev->source.subsys.u.pci.backend
 == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-unsigned long long memKB;
-
 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("VFIO PCI device assignment is not "
  "supported by this version of qemu"));
 goto error;
 }
-/* VFIO requires all of the guest's memory to be locked
- * resident, plus some amount for IO space. Alex Williamson
- * suggested adding 1GiB for IO space just to be safe (some
- * finer tuning might be nice, though).
- * In this case, the guest's memory may already be locked, but
- * it doesn't hurt to "change" the limit to the same value.
+
+/* VFIO requires all of the guest's memory to be locked resident.
+ * In this case, the guest's memory may already be locked, but it
+ * doesn't hurt to "change" the limit to the same value.
  */
-memKB = vm->def->mem.max_balloon + (1024 * 1024);
-virProcessSetMaxMemLock(vm->pid, memKB * 1024);
+vm->def->hostdevs[vm->def->nhostde

[libvirt] [PATCH 3/3] qemu: Set RLIMIT_MEMLOCK when memoryBacking/locked is used

[Jiri Denemark]

If a domain is configured to have all its memory locked, we need to set
RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory.
---
 src/qemu/qemu_command.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f902501..278d2f2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6795,6 +6795,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArgFormat(cmd, "mlock=%s",
def->mem.locked ? "on" : "off");
 }
+mlock = def->mem.locked;
 
 virCommandAddArg(cmd, "-smp");
 if (!(smp = qemuBuildSmpArgStr(def, qemuCaps)))
-- 
1.8.2.1

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

Re: [libvirt] [PATCH] domain_conf: Include the correct console alias

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 04:45:08PM +0200, Michal Privoznik wrote:
> On 28.06.2013 15:55, Daniel P. Berrange wrote:
> > On Fri, Jun 28, 2013 at 03:31:17PM +0200, Michal Privoznik wrote:
> >> For some crazy backward compatibility, a console can by just an alias to
> >> a serial device. This is detected in the XML formating function which
> >> takes the values to format from corresponding serial device. Including
> >> the device alias. This results in wrong alias being written into the XML
> >> definition:
> >>
> >> 
> >>   ...
> >>   
> >> 
> >>
> >> While holding the correct alias still in the memory, it doesn't matter.
> >> However, it starts to matter as soon as libvirtd is restarted and the
> >> (incorrect) alias is read from status file.

I don't actually see this problem at all.

Starting with a guest containing


  


  


After virsh start, it contains


  
  
  


  
  
  



The XML in $XDG_RUNTIME_DIR/libvirt/qemu/run/vm1.xml contains
exactly the same.

If i now kill libvirtd and restart it, the XML is still
reported correctly.

How do you reproduce the bug ?

> > 
> > This isn't any more correct that your previous patch. For the
> > dummy  elements, the alias *must* be copied from /
> > identical to the corresponding  device config.
> > 
> > 
> > Daniel
> > 
> 
> So what are you saying is, it's the  that has the wrong alias
> assigned? The output should be like this then?
> 
> 
>   
>   
>   
>   
> 
> 
> 
>   
>   
>   
> 

No, the alias name must match that of the device id= parameter in the
QEMU command line which is 'serial0' for a serial port.


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 glib] Fix name of gvir_config_domain_chardev_source_pty_set_path

[Daniel P. Berrange]

From: "Daniel P. Berrange" 

The method gvir_config_domain_chardev_source_pty_set_path was
accidentally called gvir_config_domain_source_pty_set_path.
This naming flaw in turn caused the introspection data to
be incorrectly generated, putting a 'source_pty_set_path'
method on the GVirConfigDomain object.
---
 libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c | 4 ++--
 libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.h | 4 ++--
 libvirt-gconfig/libvirt-gconfig.sym | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c 
b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c
index 278515c..d24385e 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c
@@ -82,8 +82,8 @@ GVirConfigDomainChardevSourcePty 
*gvir_config_domain_chardev_source_pty_new_from
 return GVIR_CONFIG_DOMAIN_CHARDEV_SOURCE_PTY(object);
 }
 
-void gvir_config_domain_source_pty_set_path(GVirConfigDomainChardevSourcePty 
*pty,
-const char *path)
+void 
gvir_config_domain_chardev_source_pty_set_path(GVirConfigDomainChardevSourcePty 
*pty,
+const char *path)
 {
 g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHARDEV_SOURCE_PTY(pty));
 
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.h 
b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.h
index 4e350e0..96e05bc 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.h
@@ -62,8 +62,8 @@ GType gvir_config_domain_chardev_source_pty_get_type(void);
 GVirConfigDomainChardevSourcePty 
*gvir_config_domain_chardev_source_pty_new(void);
 GVirConfigDomainChardevSourcePty 
*gvir_config_domain_chardev_source_pty_new_from_xml(const gchar *xml,
   
GError **error);
-void gvir_config_domain_source_pty_set_path(GVirConfigDomainChardevSourcePty 
*pty,
-const char *path);
+void 
gvir_config_domain_chardev_source_pty_set_path(GVirConfigDomainChardevSourcePty 
*pty,
+const char *path);
 
 G_END_DECLS
 
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index ccddd88..f8c7cdd 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -66,7 +66,7 @@ LIBVIRT_GCONFIG_0.0.8 {
gvir_config_domain_chardev_source_pty_get_type;
gvir_config_domain_chardev_source_pty_new;
gvir_config_domain_chardev_source_pty_new_from_xml;
-   gvir_config_domain_source_pty_set_path;
+   gvir_config_domain_chardev_source_pty_set_path;
 
gvir_config_domain_chardev_source_spicevmc_get_type;
gvir_config_domain_chardev_source_spicevmc_new;
-- 
1.8.2.1

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

Re: [libvirt] [PATCH] domain_conf: Include the correct console alias

[Michal Privoznik]

On 28.06.2013 15:55, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 03:31:17PM +0200, Michal Privoznik wrote:
>> For some crazy backward compatibility, a console can by just an alias to
>> a serial device. This is detected in the XML formating function which
>> takes the values to format from corresponding serial device. Including
>> the device alias. This results in wrong alias being written into the XML
>> definition:
>>
>> 
>>   ...
>>   
>> 
>>
>> While holding the correct alias still in the memory, it doesn't matter.
>> However, it starts to matter as soon as libvirtd is restarted and the
>> (incorrect) alias is read from status file.
>> ---
>>  src/conf/domain_conf.c | 12 +++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 011de71..61de836 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -16417,6 +16417,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>  memcpy(&console, def->serials[n], sizeof(console));
>>  console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
>>  console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
>> +memcpy(&console.info, &def->consoles[n]->info, 
>> sizeof(console.info));
>>  } else {
>>  memcpy(&console, def->consoles[n], sizeof(console));
>>  }
>> @@ -16427,11 +16428,20 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>  def->nconsoles == 0 &&
>>  def->nserials > 0) {
>>  virDomainChrDef console;
>> +char *alias = NULL;
>>  memcpy(&console, def->serials[n], sizeof(console));
>>  console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
>>  console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
>> -if (virDomainChrDefFormat(buf, &console, flags) < 0)
>> +if (console.info.alias) {
>> +if (VIR_STRDUP(alias, "console0") < 0)
>> +goto error;
>> +console.info.alias = alias;
>> +}
>> +if (virDomainChrDefFormat(buf, &console, flags) < 0) {
>> +VIR_FREE(alias);
>>  goto error;
>> +}
>> +VIR_FREE(alias);
> 
> This isn't any more correct that your previous patch. For the
> dummy  elements, the alias *must* be copied from /
> identical to the corresponding  device config.
> 
> 
> Daniel
> 

So what are you saying is, it's the  that has the wrong alias
assigned? The output should be like this then?


  
  
  
  



  
  
  


Michal

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

[libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters

[Dario Faggioli]

Although, having it depending on Xen >= 4.3 (by using the proper
libxl feature flag).

Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
"interleave" as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').

With this change, it is now possible to query the NUMA node
affinity of a running domain:

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
 IdName   State

 23F18_x64running

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode  : interleave
numa_nodeset   : 1

Signed-off-by: Dario Faggioli 
---
 src/libxl/libxl_driver.c |  138 ++
 1 file changed, 138 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53af609..9bd6d99 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -28,6 +28,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom, 
virTypedParameterPtr params,
 return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
 }
 
+/* NUMA node affinity information is available through libxl
+ * starting from Xen 4.3. */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+
+/* Number of Xen NUMA parameters */
+#define LIBXL_NUMA_NPARAM 2
+
+static int
+libxlDomainGetNumaParameters(virDomainPtr dom,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom->conn->privateData;
+libxlDomainObjPrivatePtr priv;
+virDomainObjPtr vm;
+
+libxl_bitmap nodemap;
+virBitmapPtr nodes = NULL;
+char *nodeset = NULL;
+
+int rc, ret = -1;
+int i, j;
+
+/* In Xen 4.3, it is possible to query the NUMA node affinity of a domain
+ * via libxl, but not to change it. We therefore only allow AFFECT_LIVE. */
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+libxlDriverLock(driver);
+vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
+libxlDriverUnlock(driver);
+
+if (!vm) {
+virReportError(VIR_ERR_NO_DOMAIN, "%s",
+   _("no domain with matching uuid"));
+goto cleanup;
+}
+
+if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain is not running"));
+goto cleanup;
+}
+
+priv = vm->privateData;
+
+libxl_bitmap_init(&nodemap);
+
+if ((*nparams) == 0) {
+*nparams = LIBXL_NUMA_NPARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i < LIBXL_NUMA_NPARAM && i < *nparams; i++) {
+virMemoryParameterPtr param = ¶ms[i];
+
+switch (i) {
+case 0:
+/* NUMA mode */
+
+/* Xen implements something that is is really close to numactl's
+ * 'interleave' policy (see `man 8 numactl' for details). */
+if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+VIR_TYPED_PARAM_INT,
+VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE) < 
0)
+goto cleanup;
+
+break;
+
+case 1:
+/* Node affinity */
+
+/* Let's allocate both libxl and libvirt bitmaps */
+if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
+!(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx {
+virReportOOMError();
+goto cleanup;
+}
+
+rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
+   &nodemap);
+if (rc != 0) {
+virReportSystemError(-rc, "%s",
+ _("unable to get numa affinity"));
+goto cleanup;
+}
+
+/* First, we convert libxl_bitmap into virBitmap. After that,
+ * we format virBitmap as a string that can be returned. */
+virBitmapClearAll(nodes);
+libxl_for_each_set_bit(j, nodemap) {
+if (virBitmapSetBit(nodes, j)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Node %d out of range"), j);
+goto cleanup;
+}
+}
+
+nodeset = virBitmapFormat(nodes);
+if (!nodeset && VIR_STRDUP(nodeset, "") < 0)
+goto cleanup;
+
+if (virTyp

[libvirt] [PATCH 3/4] libxl: advertise the support for VIR_TYPED_PARAM_STRING

[Dario Faggioli]

domainGetNumaParameters has a string typed parameter, hence it
is necessary for the libxl driver to support this.

This change implements the connectSupportsFeature hook for the
libxl driver, advertising that VIR_DRV_FEATURE_TYPED_PARAM_STRING
is something good to go.

Signed-off-by: Dario Faggioli 
Cc: Eric Blake 
---
 src/libxl/libxl_driver.c |   15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a3a9171..53af609 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4660,6 +4660,20 @@ libxlConnectListAllDomains(virConnectPtr conn,
 return ret;
 }
 
+/* Which features are supported by this driver? */
+static int
+libxlConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+return -1;
+
+switch (feature) {
+case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+return 1;
+default:
+return 0;
+}
+}
 
 
 static virDriver libxlDriver = {
@@ -4740,6 +4754,7 @@ static virDriver libxlDriver = {
 .connectDomainEventRegisterAny = libxlConnectDomainEventRegisterAny, /* 
0.9.0 */
 .connectDomainEventDeregisterAny = libxlConnectDomainEventDeregisterAny, 
/* 0.9.0 */
 .connectIsAlive = libxlConnectIsAlive, /* 0.9.8 */
+.connectSupportsFeature = libxlConnectSupportsFeature, /* 1.1.1 */
 };
 
 static virStateDriver libxlStateDriver = {

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

[libvirt] [PATCH 2/4] libxl: implement per NUMA node free memory reporting

[Dario Faggioli]

By providing the implementation of nodeGetCellsFreeMemory for
the driver. This is all just a matter of properly formatting, in
a way that libvirt like, what Xen provides via libxl_get_numainfo().

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// freecell --all
0:  25004 KiB
1: 105848 KiB

Total: 130852 KiB

Signed-off-by: Dario Faggioli 
---
 src/libxl/libxl_driver.c |   46 ++
 1 file changed, 46 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9f52394..a3a9171 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4098,6 +4098,51 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
 }
 
 static int
+libxlNodeGetCellsFreeMemory(virConnectPtr conn,
+unsigned long long *freeMems,
+int startCell,
+int maxCells)
+{
+int n, lastCell, numCells;
+int ret = -1, nr_nodes = 0;
+libxl_numainfo *numa_info = NULL;
+libxlDriverPrivatePtr driver = conn->privateData;
+
+if (virNodeGetCellsFreeMemoryEnsureACL(conn) < 0)
+return -1;
+
+/* Early failure is probably worth just a warning */
+numa_info = libxl_get_numainfo(driver->ctx, &nr_nodes);
+if (numa_info == NULL || nr_nodes == 0) {
+VIR_WARN("libxl_get_numainfo failed to retrieve NUMA data");
+return 0;
+}
+
+/* Check/sanitize the cell range */
+if (startCell > nr_nodes) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("start cell %d out of range (0-%d)"),
+   startCell, nr_nodes);
+goto cleanup;
+}
+lastCell = startCell + maxCells - 1;
+if (lastCell > nr_nodes)
+lastCell = nr_nodes;
+
+for (numCells = 0, n = startCell; n <= lastCell; n++) {
+if (numa_info[n].size == LIBXL_NUMAINFO_INVALID_ENTRY)
+freeMems[numCells++] = 0;
+else
+freeMems[numCells++] = numa_info[n].free;
+}
+ret = numCells;
+
+cleanup:
+libxl_numainfo_list_free(numa_info, nr_nodes);
+return ret;
+}
+
+static int
 libxlConnectDomainEventRegister(virConnectPtr conn,
 virConnectDomainEventCallback callback, void 
*opaque,
 virFreeCallback freecb)
@@ -4683,6 +4728,7 @@ static virDriver libxlDriver = {
 .domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 
0.9.0 */
 .domainSetSchedulerParametersFlags = 
libxlDomainSetSchedulerParametersFlags, /* 0.9.2 */
 .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
+.nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
 .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
 .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 
0.9.0 */
 .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */

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

[libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting

[Dario Faggioli]

Starting from Xen 4.2, libxl has all the bits and pieces are in
place for retrieving an adequate amount of information about the
host NUMA topology. It is therefore possible, after a bit of
shuffling, to arrange those information in the way libvirt wants
to present them to the outside world.

Therefore, with this patch, the  section of the host
capabilities is properly populated, when running on Xen, so that
we can figure out whether or not we're running on a NUMA host,
and what its characteristics are.

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities

  



  

  6291456
  








  


  6881280
  








  

  

  
  

Signed-off-by: Dario Faggioli 
---
 src/libxl/libxl_conf.c |  128 
 1 file changed, 127 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e170357..2a9bcf0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -163,6 +163,8 @@ libxlBuildCapabilities(virArch hostarch,
 static virCapsPtr
 libxlMakeCapabilitiesInternal(virArch hostarch,
   libxl_physinfo *phy_info,
+  libxl_numainfo *numa_info, int nr_nodes,
+  libxl_cputopology *cpu_topo, int nr_cpus,
   char *capabilities)
 {
 char *str, *token;
@@ -173,6 +175,12 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
 int host_pae = 0;
 struct guest_arch guest_archs[32];
 int nr_guest_archs = 0;
+
+/* For building NUMA capabilities */
+virCapsHostNUMACellCPUPtr *cpus = NULL;
+int *nr_cpus_node = NULL;
+bool numa_failed = false;
+
 virCapsPtr caps = NULL;
 
 memset(guest_archs, 0, sizeof(guest_archs));
@@ -280,6 +288,100 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
nr_guest_archs)) == NULL)
 goto no_memory;
 
+/* What about NUMA? */
+if (!numa_info || !cpu_topo)
+return caps;
+
+if (VIR_ALLOC_N(cpus, nr_nodes))
+goto no_memory;
+memset(cpus, 0, sizeof(cpus) * nr_nodes);
+
+if (VIR_ALLOC_N(nr_cpus_node, nr_nodes)) {
+VIR_FREE(cpus);
+goto no_memory;
+}
+memset(nr_cpus_node, 0, sizeof(nr_cpus_node) * nr_nodes);
+
+/* For each node, prepare a list of CPUs belonging to that node */
+for (i = 0; i < nr_cpus; i++) {
+int node = cpu_topo[i].node;
+
+if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+continue;
+
+nr_cpus_node[node]++;
+
+if (nr_cpus_node[node] == 1) {
+if (VIR_ALLOC(cpus[node]) < 0) {
+numa_failed = true;
+goto cleanup;
+}
+}
+else {
+if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node]) < 0) {
+numa_failed = true;
+goto cleanup;
+}
+}
+
+/* Mapping between what libxl tells and what libvirt wants */
+cpus[node][nr_cpus_node[node]-1].id = i;
+cpus[node][nr_cpus_node[node]-1].socket_id = cpu_topo[i].socket;
+cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core;
+cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
+
+if (!cpus[node][nr_cpus_node[node]-1].siblings) {
+virReportOOMError();
+numa_failed = true;
+goto cleanup;
+}
+}
+
+/* Let's now populate the siblings bitmaps */
+for (i = 0; i < nr_cpus; i++) {
+int j, node = cpu_topo[i].node;
+
+if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+continue;
+
+for (j = 0; j < nr_cpus_node[node]; j++) {
+if (cpus[node][j].core_id == cpu_topo[i].core)
+ignore_value(virBitmapSetBit(cpus[node][j].siblings, i));
+}
+}
+
+for (i = 0; i < nr_nodes; i++) {
+if (numa_info[i].size == LIBXL_NUMAINFO_INVALID_ENTRY)
+continue;
+
+if (virCapabilitiesAddHostNUMACell(caps, i, nr_cpus_node[i],
+   numa_info[i].size / 1024,
+   cpus[i]) < 0) {
+virCapabilitiesClearHostNUMACellCPUTopology(cpus[i],
+nr_cpus_node[i]);
+numa_failed = true;
+goto cleanup;
+}
+
+/* This is safe, as the CPU list is now stored in the NUMA cell */
+cpus[i] = NULL;
+}
+
+cleanup:
+if (numa_failed) {
+/* Looks like something went wrong. Well, that's bad, but probably
+ * not enough to break the whole driver, so we lo

[libvirt] [PATCH 0/4] libxl: implement some chuncks of the NUMA interface

[Dario Faggioli]

Hi Jim, Everyone,

First patch series, so maybe a small introduction is required: I'm Dario and I
work for Citrix on improving the NUMA support of Xen.

This patch series implements some of the missing bits and pieces, in the libxl
driver, regarding obtaining per-host and per-domain NUMA related information.
It's not the full libvirt NUMA interface, since we don't have all we would need
for that in Xen yet (we will in the next version, 4.4), but it's certainly
better than having nothing! :-)

Basically, I'm enhancing capability reporting, to cover NUMA topology (patch
01), and I'm implementing  nodeGetCellsFreeMemory (patch 02) and
virDomainGetNumaParameters (patch 04) for the libxl driver. This last one
requires at least Xen 4.3, so I put the implementation within the proper
#ifdef-ery.

What I'm really not sure about is patch 03, which is something I need if I want
patch 04 to function properly. Basically it is about advertising that the libxl
driver supports VIR_TYPED_PARAM_STRING. I looked at how that is done in the
qemu driver, but I'm not entirely sure I completely understood the logic behind
it, so, please, tell me if I missed or misinterpreted anything!  In particular,
should I have added more of those "flags &= ~VIR_TYPED_PARAM_STRING_OKAY;"
statements, as it happens in the qemu driver? If yes, in which functions?

Finally, allow me to say that it was a while that I wanted start hacking a bit
on libvirt.  I'm really glad I've eventually been able to do so, and I
definitely plan to continue (with particular focus on NUMA related stuff).

Comments are of course more than welcome. :-)

Thanks and Regards,
Dario

---

Dario Faggioli (4):
  libxl: implement NUMA capabilities reporting
  libxl: implement per NUMA node free memory reporting
  libxl: advertise the support for VIR_TYPED_PARAM_STRING
  libxl: implement virDomainGetNumaParameters


 src/libxl/libxl_conf.c   |  128 +-
 src/libxl/libxl_driver.c |  199 ++
 2 files changed, 326 insertions(+), 1 deletion(-)

-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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

Re: [libvirt] [Libvirt-announce] Candidate release 2 for libvirt-1.1.0

[Justin Clift]

On 28/06/2013, at 3:05 PM, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 03:01:58PM +0100, Justin Clift wrote:

>> Googling shows IF_MAXUNIT in context of FreeBSD stuff.  Not sure
>> of the right approach here.  eg exclude compiling this on OSX,
>> or alternatively make-it-work on OSX.
> 
> Yes, this is from code that is only designed to work on FreeBSD. I
> guess the OS-X BSD base isn't close enough to support this properly.
> 
> Try this patch
> 
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index 265676c..4e2f32a 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -275,7 +275,7 @@ cleanup:
> VIR_FORCE_CLOSE(fd);
> return ret;
> }
> -#elif defined(SIOCIFCREATE2) && defined(SIOCIFDESTROY)
> +#elif defined(SIOCIFCREATE2) && defined(SIOCIFDESTROY) && defined(IF_MAXUNIT)
> int virNetDevTapCreate(char **ifname,
>int *tapfd,
>int tapfdSize,


Thanks Dan, that works well.  Libvirt is now compiling, and virsh
seems to be working happily (with a minimal amount of testing
anyway). :)

Regards and best wishes,

Justin Clift

--
Open Source and Standards @ Red Hat

twitter.com/realjustinclift


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

Re: [libvirt] [Libvirt-announce] Candidate release 2 for libvirt-1.1.0

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 03:01:58PM +0100, Justin Clift wrote:
> On 27/06/2013, at 5:01 PM, Daniel Veillard wrote:
> >   I have just tagged the release candidate 2 in git and sent a tarball
> > to the usual place (rpms are coming):
> >   ftp://libvirt.org/libvirt/
> 
> Sorry for the delay, only just now remembered to test this. :)
> 
> Compilation is failing on OSX 10.7 x64 with:
> 
> **
>   CC libvirt_util_la-virnetdevveth.lo
>   CC libvirt_util_la-virnetdevvlan.lo
>   CC libvirt_util_la-virnetdevvportprofile.lo
>   CC libvirt_util_la-virnetlink.lo
>   CC libvirt_util_la-virnodesuspend.lo
>   CC libvirt_util_la-virnuma.lo
> util/virnetdevtap.c:315:26: error: use of undeclared identifier 'IF_MAXUNIT'
> for (i = 0; i <= IF_MAXUNIT; i++) {

[snip]

> Googling shows IF_MAXUNIT in context of FreeBSD stuff.  Not sure
> of the right approach here.  eg exclude compiling this on OSX,
> or alternatively make-it-work on OSX.

Yes, this is from code that is only designed to work on FreeBSD. I
guess the OS-X BSD base isn't close enough to support this properly.

Try this patch

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 265676c..4e2f32a 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -275,7 +275,7 @@ cleanup:
 VIR_FORCE_CLOSE(fd);
 return ret;
 }
-#elif defined(SIOCIFCREATE2) && defined(SIOCIFDESTROY)
+#elif defined(SIOCIFCREATE2) && defined(SIOCIFDESTROY) && defined(IF_MAXUNIT)
 int virNetDevTapCreate(char **ifname,
int *tapfd,
int tapfdSize,


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

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

Re: [libvirt] [Libvirt-announce] Candidate release 2 for libvirt-1.1.0

[Justin Clift]

On 27/06/2013, at 5:01 PM, Daniel Veillard wrote:
>   I have just tagged the release candidate 2 in git and sent a tarball
> to the usual place (rpms are coming):
>   ftp://libvirt.org/libvirt/

Sorry for the delay, only just now remembered to test this. :)

Compilation is failing on OSX 10.7 x64 with:

**
  CC libvirt_util_la-virnetdevveth.lo
  CC libvirt_util_la-virnetdevvlan.lo
  CC libvirt_util_la-virnetdevvportprofile.lo
  CC libvirt_util_la-virnetlink.lo
  CC libvirt_util_la-virnodesuspend.lo
  CC libvirt_util_la-virnuma.lo
util/virnetdevtap.c:315:26: error: use of undeclared identifier 'IF_MAXUNIT'
for (i = 0; i <= IF_MAXUNIT; i++) {
 ^
1 error generated.
make[3]: *** [libvirt_util_la-virnetdevtap.lo] Error 1
make[3]: *** Waiting for unfinished jobs
[all] Error 2
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
**

There doesn't seem to be a definition for IF_MAXUNIT on OSX (10.7
anyway):

  $ grep -ri IF_MAXUNIT /usr/include/
  $

Interestingly, it's also not on a RHEL 6.4 box locally here
either:

  $ grep -ri "IF_MAXUNIT" /usr/include/
  $

Though, it's possible that box is missing required devel packages
for libvirt. (unsure)

Googling shows IF_MAXUNIT in context of FreeBSD stuff.  Not sure
of the right approach here.  eg exclude compiling this on OSX,
or alternatively make-it-work on OSX.

Hope that helps. :)

+ Justin

--
Open Source and Standards @ Red Hat

twitter.com/realjustinclift

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

Re: [libvirt] [PATCH] domain_conf: Include the correct console alias

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 03:31:17PM +0200, Michal Privoznik wrote:
> For some crazy backward compatibility, a console can by just an alias to
> a serial device. This is detected in the XML formating function which
> takes the values to format from corresponding serial device. Including
> the device alias. This results in wrong alias being written into the XML
> definition:
> 
> 
>   ...
>   
> 
> 
> While holding the correct alias still in the memory, it doesn't matter.
> However, it starts to matter as soon as libvirtd is restarted and the
> (incorrect) alias is read from status file.
> ---
>  src/conf/domain_conf.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 011de71..61de836 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -16417,6 +16417,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>  memcpy(&console, def->serials[n], sizeof(console));
>  console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
>  console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
> +memcpy(&console.info, &def->consoles[n]->info, 
> sizeof(console.info));
>  } else {
>  memcpy(&console, def->consoles[n], sizeof(console));
>  }
> @@ -16427,11 +16428,20 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>  def->nconsoles == 0 &&
>  def->nserials > 0) {
>  virDomainChrDef console;
> +char *alias = NULL;
>  memcpy(&console, def->serials[n], sizeof(console));
>  console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
>  console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
> -if (virDomainChrDefFormat(buf, &console, flags) < 0)
> +if (console.info.alias) {
> +if (VIR_STRDUP(alias, "console0") < 0)
> +goto error;
> +console.info.alias = alias;
> +}
> +if (virDomainChrDefFormat(buf, &console, flags) < 0) {
> +VIR_FREE(alias);
>  goto error;
> +}
> +VIR_FREE(alias);

This isn't any more correct that your previous patch. For the
dummy  elements, the alias *must* be copied from /
identical to the corresponding  device config.


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] domain_conf: Include the correct console alias

[Michal Privoznik]

For some crazy backward compatibility, a console can by just an alias to
a serial device. This is detected in the XML formating function which
takes the values to format from corresponding serial device. Including
the device alias. This results in wrong alias being written into the XML
definition:


  ...
  


While holding the correct alias still in the memory, it doesn't matter.
However, it starts to matter as soon as libvirtd is restarted and the
(incorrect) alias is read from status file.
---
 src/conf/domain_conf.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 011de71..61de836 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16417,6 +16417,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 memcpy(&console, def->serials[n], sizeof(console));
 console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
 console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+memcpy(&console.info, &def->consoles[n]->info, 
sizeof(console.info));
 } else {
 memcpy(&console, def->consoles[n], sizeof(console));
 }
@@ -16427,11 +16428,20 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 def->nconsoles == 0 &&
 def->nserials > 0) {
 virDomainChrDef console;
+char *alias = NULL;
 memcpy(&console, def->serials[n], sizeof(console));
 console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
 console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
-if (virDomainChrDefFormat(buf, &console, flags) < 0)
+if (console.info.alias) {
+if (VIR_STRDUP(alias, "console0") < 0)
+goto error;
+console.info.alias = alias;
+}
+if (virDomainChrDefFormat(buf, &console, flags) < 0) {
+VIR_FREE(alias);
 goto error;
+}
+VIR_FREE(alias);
 }
 
 for (n = 0; n < def->nchannels; n++)
-- 
1.8.1.5

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

[libvirt] Bug 979411 - virsh migration copy-storage-all fails with "Unable to read from monitor: Connection reset by peer"

[chandrashekar shastri]

Hi,

I have filed a bug for virsh migration copy-storage-all:

Bug 979411 - virsh migration copy-storage-all fails with "Unable to read 
from monitor: Connection reset by peer"


Earlier I had filed this bug in launched:

Bug #1192499 : virsh migration copy-storage-all fails with "Unable to 
read from monitor: Connection reset by peer"


Thanks,
Chandrashekar Shastri

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

Re: [libvirt] Candidate release 2 for libvirt-1.1.0

[Doug Goldstein]

On Thu, Jun 27, 2013 at 9:46 PM, Doug Goldstein  wrote:

>
>
> On Thu, Jun 27, 2013 at 11:01 AM, Daniel Veillard wrote:
>
>>I have just tagged the release candidate 2 in git and sent a tarball
>> to the usual place (rpms are coming):
>>ftp://libvirt.org/libvirt/
>>
>> This includes the end of the patch set from Laine, and hopefully
>> it won't require too many other patches. I tried it and it doesn't
>> look obviously broken to me, please give it a try too, especially
>> for portability :-)
>> If all goes well the final release should be next Monday !
>>
>>   thanks !
>>
>> Daniel
>>
>> --
>> Daniel Veillard  | Open Source and Standards, Red Hat
>> veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
>> http://veillard.com/ | virtualization library  http://libvirt.org/
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>
> Not sure if this is a regression yet, but migrations are failing for me on
> git (currently running both machines at the v1.1.0_rc2 tag)
>
> virsh migrate --live --p2p --persistent --copy-storage-all expo
> qemu+tcp://cocacola/system
> error: Failed to open file '/var/lib/libvirt/images/expo.img': No such
> file or directory
>
> The error message is from the remote that the file isn't there. I was
> under the impression that a NBD starts up and streams the data over. It
> gives me an error to hint that if I can include --tunnelled in the options.
>
> Using qemu 1.4.2 for reference (its Cole's patchset for 1.4.2 from
> Fedora's qemu.git)
>
> --
> Doug Goldstein
>

Sorry for the noise. I apparently have an instance still running Michal's
patchset that provided that functionality on one of my machines without
realizing it.

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[David Maciejak]

On Fri, Jun 28, 2013 at 1:55 PM, Michal Novotny  wrote:

>
> On 06/28/2013 01:54 PM, Peter Krempa wrote:
> > On 06/28/13 13:50, Michal Novotny wrote:
> >> On 06/28/2013 01:48 PM, Daniel P. Berrange wrote:
> >>> On Fri, Jun 28, 2013 at 01:43:44PM +0200, Michal Novotny wrote:
>  if libvirt doesn't have the tokenizer support yet, it may be a good
> RFE
>  as I believe it could be really useful ;-)
> 
>  Peter, do you know about anything libvirt supports to tokenize string?
> >>> We have virStringSplit for tokenizing strings which have a fixed
> >>> separator.
> >>>
> >>>
> >>> Daniel
> >> That sounds good, however what about splitting the function to 2
> >> separate functions, one accepting the second parameter as the separator,
> >> called e.g. virStringSplitBy() and second just calling the first one
> >> with the fixed separator?
> >>
> >> Wouldn't it be better?
> > I think the current state is more than sufficient:
> >
> > /**
> >  * virStringSplit:
> >  * @string: a string to split
> >  * @delim: a string which specifies the places at which to split
> >  * the string. The delimiter is not included in any of the resulting
> >  * strings, unless @max_tokens is reached.
> >  * @max_tokens: the maximum number of pieces to split @string into.
> >  * If this is 0, the string is split completely.
> >  *
> >  * Splits a string into a maximum of @max_tokens pieces, using the given
> >  * @delim. If @max_tokens is reached, the remainder of @string is
> >  * appended to the last token.
> >  *
> >  * As a special case, the result of splitting the empty string "" is an
> empty
> >  * vector, not a vector containing a single string. The reason for this
> >  * special case is that being able to represent a empty vector is
> typically
> >  * more useful than consistent handling of empty elements. If you do need
> >  * to represent empty elements, you'll need to check for the empty string
> >  * before calling virStringSplit().
> >  *
> >  * Return value: a newly-allocated NULL-terminated array of strings. Use
> >  *virStringFreeList() to free it.
> >  */
> > char **virStringSplit(const char *string,
> >   const char *delim,
> >   size_t max_tokens)
>
> Ah, then I misunderstood the "fixed separator" thing ;-) I was thinking
> the delimiter it fixed. This one looks good ;-)
>
> David, you could use virStringSplit() instead ;-)
>
>
I will wait for Peter's patch ;)


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

Re: [libvirt] [PATCH] Replace use of 'in_addr_t' with 'struct in_addr'

[Jiri Denemark]

On Fri, Jun 28, 2013 at 12:45:14 +0100, Daniel Berrange wrote:
> From: "Daniel P. Berrange" 
> 
> The 'in_addr_t' typedef is not present in Mingw64 headers.
> Instead we can use the more portable 'struct in_addr' and
> then access its 's_addr' field.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/util/virsocketaddr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

ACK

/me slaps himself for suggesting in_addr_t and IN6_IS_ADDR_UNSPECIFIED
:-)

Jirka

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[Michal Novotny]

On 06/28/2013 01:54 PM, Peter Krempa wrote:
> On 06/28/13 13:50, Michal Novotny wrote:
>> On 06/28/2013 01:48 PM, Daniel P. Berrange wrote:
>>> On Fri, Jun 28, 2013 at 01:43:44PM +0200, Michal Novotny wrote:
 if libvirt doesn't have the tokenizer support yet, it may be a good RFE
 as I believe it could be really useful ;-)

 Peter, do you know about anything libvirt supports to tokenize string?
>>> We have virStringSplit for tokenizing strings which have a fixed
>>> separator.
>>>
>>>
>>> Daniel
>> That sounds good, however what about splitting the function to 2
>> separate functions, one accepting the second parameter as the separator,
>> called e.g. virStringSplitBy() and second just calling the first one
>> with the fixed separator?
>>
>> Wouldn't it be better?
> I think the current state is more than sufficient:
>
> /**
>  * virStringSplit:
>  * @string: a string to split
>  * @delim: a string which specifies the places at which to split
>  * the string. The delimiter is not included in any of the resulting
>  * strings, unless @max_tokens is reached.
>  * @max_tokens: the maximum number of pieces to split @string into.
>  * If this is 0, the string is split completely.
>  *
>  * Splits a string into a maximum of @max_tokens pieces, using the given
>  * @delim. If @max_tokens is reached, the remainder of @string is
>  * appended to the last token.
>  *
>  * As a special case, the result of splitting the empty string "" is an empty
>  * vector, not a vector containing a single string. The reason for this
>  * special case is that being able to represent a empty vector is typically
>  * more useful than consistent handling of empty elements. If you do need
>  * to represent empty elements, you'll need to check for the empty string
>  * before calling virStringSplit().
>  *
>  * Return value: a newly-allocated NULL-terminated array of strings. Use
>  *virStringFreeList() to free it.
>  */
> char **virStringSplit(const char *string,
>   const char *delim,
>   size_t max_tokens)

Ah, then I misunderstood the "fixed separator" thing ;-) I was thinking
the delimiter it fixed. This one looks good ;-)

David, you could use virStringSplit() instead ;-)

Michal


-- 
Michal Novotny , RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 01:50:08PM +0200, Michal Novotny wrote:
> 
> On 06/28/2013 01:48 PM, Daniel P. Berrange wrote:
> > On Fri, Jun 28, 2013 at 01:43:44PM +0200, Michal Novotny wrote:
> >> if libvirt doesn't have the tokenizer support yet, it may be a good RFE
> >> as I believe it could be really useful ;-)
> >>
> >> Peter, do you know about anything libvirt supports to tokenize string?
> > We have virStringSplit for tokenizing strings which have a fixed
> > separator.
> >
> >
> > Daniel
> 
> That sounds good, however what about splitting the function to 2
> separate functions, one accepting the second parameter as the separator,
> called e.g. virStringSplitBy() and second just calling the first one
> with the fixed separator?
> 
> Wouldn't it be better?

That sounds like overkill to me unless we have a clear need for it
somewhere in our code.


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] add support for libssh2 password from auth file

[Peter Krempa]

On 06/28/13 13:50, Michal Novotny wrote:
> 
> On 06/28/2013 01:48 PM, Daniel P. Berrange wrote:
>> On Fri, Jun 28, 2013 at 01:43:44PM +0200, Michal Novotny wrote:
>>> if libvirt doesn't have the tokenizer support yet, it may be a good RFE
>>> as I believe it could be really useful ;-)
>>>
>>> Peter, do you know about anything libvirt supports to tokenize string?
>> We have virStringSplit for tokenizing strings which have a fixed
>> separator.
>>
>>
>> Daniel
> 
> That sounds good, however what about splitting the function to 2
> separate functions, one accepting the second parameter as the separator,
> called e.g. virStringSplitBy() and second just calling the first one
> with the fixed separator?
> 
> Wouldn't it be better?

I think the current state is more than sufficient:

/**
 * virStringSplit:
 * @string: a string to split
 * @delim: a string which specifies the places at which to split
 * the string. The delimiter is not included in any of the resulting
 * strings, unless @max_tokens is reached.
 * @max_tokens: the maximum number of pieces to split @string into.
 * If this is 0, the string is split completely.
 *
 * Splits a string into a maximum of @max_tokens pieces, using the given
 * @delim. If @max_tokens is reached, the remainder of @string is
 * appended to the last token.
 *
 * As a special case, the result of splitting the empty string "" is an empty
 * vector, not a vector containing a single string. The reason for this
 * special case is that being able to represent a empty vector is typically
 * more useful than consistent handling of empty elements. If you do need
 * to represent empty elements, you'll need to check for the empty string
 * before calling virStringSplit().
 *
 * Return value: a newly-allocated NULL-terminated array of strings. Use
 *virStringFreeList() to free it.
 */
char **virStringSplit(const char *string,
  const char *delim,
  size_t max_tokens)


> 
> Michal
> 

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

Re: [libvirt] [PATCH] Allow RO connections to interface udev backend

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 06:43:37AM -0500, Doug Goldstein wrote:
> On Fri, Jun 28, 2013 at 2:49 AM, Ján Tomko  wrote:
> 
> > On 06/28/2013 03:48 AM, Doug Goldstein wrote:
> > > The udev based interface backend did not allow querying data over a
> > > read-only connection which is different than how the netcf backend
> > > operates. This brings the behavior inline with the default, netcf
> > > backend.
> > > ---
> > >  src/interface/interface_backend_udev.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> >
> > ACK
> >
> > Jan
> >
> >
> Ok to push for 1.1.0? This really should be included IMHO to unbreak the
> API.

ACK

Minimal risk, so fine to push.

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] add support for libssh2 password from auth file

[Michal Novotny]

On 06/28/2013 01:48 PM, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 01:43:44PM +0200, Michal Novotny wrote:
>> if libvirt doesn't have the tokenizer support yet, it may be a good RFE
>> as I believe it could be really useful ;-)
>>
>> Peter, do you know about anything libvirt supports to tokenize string?
> We have virStringSplit for tokenizing strings which have a fixed
> separator.
>
>
> Daniel

That sounds good, however what about splitting the function to 2
separate functions, one accepting the second parameter as the separator,
called e.g. virStringSplitBy() and second just calling the first one
with the fixed separator?

Wouldn't it be better?

Michal

-- 
Michal Novotny , RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 01:43:44PM +0200, Michal Novotny wrote:
> if libvirt doesn't have the tokenizer support yet, it may be a good RFE
> as I believe it could be really useful ;-)
> 
> Peter, do you know about anything libvirt supports to tokenize string?

We have virStringSplit for tokenizing strings which have a fixed
separator.


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] Replace use of 'in_addr_t' with 'struct in_addr'

[Daniel P. Berrange]

From: "Daniel P. Berrange" 

The 'in_addr_t' typedef is not present in Mingw64 headers.
Instead we can use the more portable 'struct in_addr' and
then access its 's_addr' field.

Signed-off-by: Daniel P. Berrange 
---
 src/util/virsocketaddr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index 2666574..80b5c0b 100644
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -235,10 +235,10 @@ virSocketAddrIsPrivate(const virSocketAddrPtr addr)
 bool
 virSocketAddrIsWildcard(const virSocketAddrPtr addr)
 {
-in_addr_t tmp = INADDR_ANY;
+struct in_addr tmp = { .s_addr = INADDR_ANY };
 switch (addr->data.stor.ss_family) {
 case AF_INET:
-return memcmp(&addr->data.inet4.sin_addr.s_addr, &tmp,
+return memcmp(&addr->data.inet4.sin_addr.s_addr, &tmp.s_addr,
   sizeof(addr->data.inet4.sin_addr.s_addr)) == 0;
 case AF_INET6:
 return IN6_IS_ADDR_UNSPECIFIED(&addr->data.inet6.sin6_addr);
-- 
1.8.2.1

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

Re: [libvirt] [PATCH] Allow RO connections to interface udev backend

[Doug Goldstein]

On Fri, Jun 28, 2013 at 2:49 AM, Ján Tomko  wrote:

> On 06/28/2013 03:48 AM, Doug Goldstein wrote:
> > The udev based interface backend did not allow querying data over a
> > read-only connection which is different than how the netcf backend
> > operates. This brings the behavior inline with the default, netcf
> > backend.
> > ---
> >  src/interface/interface_backend_udev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> ACK
>
> Jan
>
>
Ok to push for 1.1.0? This really should be included IMHO to unbreak the
API.

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[Michal Novotny]

On 06/28/2013 01:41 PM, David Maciejak wrote:
>
>
>
> On Fri, Jun 28, 2013 at 1:03 PM, Michal Novotny  > wrote:
>
>
> On 06/28/2013 12:28 PM, Peter Krempa wrote:
> > On 06/27/13 14:31, David Maciejak wrote:
> >> Hi,
> >>
> >> I was discussing with Daniel about the best way to pass the ssh
> password
> >> when using such kind of uri:
> >> 'xen+libssh2://root@192.168.0.10?sshauth=password
> 
> >> '
> >>
> >> As it seems passing the password in the uri is not a good
> option, maybe we
> >> can grab it from auth conf ? it seems it's not the case as now
> (tell me
> >> if i am wrong).
> > I was planing on doing this stuff, but never managed to finish this.
> >
> >> So enclosed a patch to add this feature.
> >>
> >> As you can see in virnetclient.c there is no virAuthGetPassword
> call, so
> >> the authfile is never used.
> >>
> >> The patch enclosed is modifying the function prototype to add
> >> virConnectPtr parameter (so remote_driver.c
> virNetClientNewLibSSH2 call
> >> has to be updated too and the corresponding .h too).
> >>
> >> Once we have access to virConnectPtr, as you will see in the
> patch we
> >> can check if authMethods is set to 'password' and grab the
> password from
> >> auth file by calling virAuthGetPassword.
> >>
> >>
> > please use git format-patch and send-email in the future, it makes
> > reviewing easier.
> >
> > See the attached patch for the review.
> >
> > Peter
> >
> >
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com 
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
> Hi all,
> Peter, you could have reformatted it for better replies on
> separate code
> hunks ;-) Like I did now ;-)
>
>
> Thanks for the review guys.
> ...
>
>
> I suggest tokenizing it after commas, like I'm doing for my
> personal projects
>
>
>
> I agree with Michal,  using strstr instead of strcmp has also some
> drawbacks.

Hi David,
if libvirt doesn't have the tokenizer support yet, it may be a good RFE
as I believe it could be really useful ;-)

Peter, do you know about anything libvirt supports to tokenize string?

Thanks,
Michal
>
>
> regards,
> david

-- 
Michal Novotny , RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[David Maciejak]

On Fri, Jun 28, 2013 at 1:03 PM, Michal Novotny  wrote:

>
> On 06/28/2013 12:28 PM, Peter Krempa wrote:
> > On 06/27/13 14:31, David Maciejak wrote:
> >> Hi,
> >>
> >> I was discussing with Daniel about the best way to pass the ssh password
> >> when using such kind of uri:
> >> 'xen+libssh2://root@192.168.0.10?sshauth=password
> >> '
> >>
> >> As it seems passing the password in the uri is not a good option, maybe
> we
> >> can grab it from auth conf ? it seems it's not the case as now (tell me
> >> if i am wrong).
> > I was planing on doing this stuff, but never managed to finish this.
> >
> >> So enclosed a patch to add this feature.
> >>
> >> As you can see in virnetclient.c there is no virAuthGetPassword call, so
> >> the authfile is never used.
> >>
> >> The patch enclosed is modifying the function prototype to add
> >> virConnectPtr parameter (so remote_driver.c virNetClientNewLibSSH2 call
> >> has to be updated too and the corresponding .h too).
> >>
> >> Once we have access to virConnectPtr, as you will see in the patch we
> >> can check if authMethods is set to 'password' and grab the password from
> >> auth file by calling virAuthGetPassword.
> >>
> >>
> > please use git format-patch and send-email in the future, it makes
> > reviewing easier.
> >
> > See the attached patch for the review.
> >
> > Peter
> >
> >
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
> Hi all,
> Peter, you could have reformatted it for better replies on separate code
> hunks ;-) Like I did now ;-)
>
>
Thanks for the review guys.
...

>
> I suggest tokenizing it after commas, like I'm doing for my personal
> projects
>


I agree with Michal,  using strstr instead of strcmp has also some
drawbacks.


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

Re: [libvirt] [PATCH] qemu: Restored original console alias

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 01:11:40PM +0200, Michal Privoznik wrote:
> Because of some crazy backward compatibility, console device is in
> some cases just an alias to a serial device. This means, in the process
> of generating XML description of a domain, all the interesting info is
> taken from corresponding serial device, if that's the case. Including
> the device alias. That means, we produce:
> 
>   ...
>   
> 
> 
> (notice the assigned alias)
> 
> Maybe this is okay, maybe its wrong either. Anyway, later, when libvirtd
> restarts, and we parse the state XML file, we read the wrong alias back.
> Hence, the internal representation is different to the state it was in
> prior the libvirtd restart.
> ---
>  src/qemu/qemu_domain.c | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8d79066..96d88ec 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -804,6 +804,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>  dev->data.chr->source.data.nix.listen = true;
>  }
>  
> +/* For some really crazy back compat in virDomainDefFormatInternal we 
> must
> + * restore the original console alias. For hvm domains, we are formatting
> + * a dummy console device (based on a serial device which it refers to)
> + * instead of the original one.  That means the device aliases in memory
> + * and in the formatted XML are not in sync. While in memory we still 
> have
> + * 'consoleN', in the formatted XML we have 'serialN'. */
> +if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
> +dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
> +dev->data.chr->targetType == 
> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL &&
> +STREQ(def->os.type, "hvm")) {
> +int id;
> +char *alias = dev->data.chr->info.alias;
> +const char *serial_alias = "serial";
> +
> +if (alias && STRPREFIX(alias, serial_alias)) {
> +alias += strlen(serial_alias);
> +
> +if (virStrToLong_i(alias, NULL, 10, &id) < 0)
> +goto cleanup;
> +
> +VIR_FREE(dev->data.chr->info.alias);
> +if (virAsprintf(&dev->data.chr->info.alias, "console%d", id) < 
> 0) {

The  names in the XML must match the 'id' values used for the
devices created in QEMU. So if the  device is a clone of the
primary serial device, then it is not correct to be changing 'serial0'
to 'console0' here.


Dnaiel
-- 
|: 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] qemu: Restored original console alias

[Michal Privoznik]

Because of some crazy backward compatibility, console device is in
some cases just an alias to a serial device. This means, in the process
of generating XML description of a domain, all the interesting info is
taken from corresponding serial device, if that's the case. Including
the device alias. That means, we produce:

  ...
  


(notice the assigned alias)

Maybe this is okay, maybe its wrong either. Anyway, later, when libvirtd
restarts, and we parse the state XML file, we read the wrong alias back.
Hence, the internal representation is different to the state it was in
prior the libvirtd restart.
---
 src/qemu/qemu_domain.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8d79066..96d88ec 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -804,6 +804,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 dev->data.chr->source.data.nix.listen = true;
 }
 
+/* For some really crazy back compat in virDomainDefFormatInternal we must
+ * restore the original console alias. For hvm domains, we are formatting
+ * a dummy console device (based on a serial device which it refers to)
+ * instead of the original one.  That means the device aliases in memory
+ * and in the formatted XML are not in sync. While in memory we still have
+ * 'consoleN', in the formatted XML we have 'serialN'. */
+if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
+dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL 
&&
+STREQ(def->os.type, "hvm")) {
+int id;
+char *alias = dev->data.chr->info.alias;
+const char *serial_alias = "serial";
+
+if (alias && STRPREFIX(alias, serial_alias)) {
+alias += strlen(serial_alias);
+
+if (virStrToLong_i(alias, NULL, 10, &id) < 0)
+goto cleanup;
+
+VIR_FREE(dev->data.chr->info.alias);
+if (virAsprintf(&dev->data.chr->info.alias, "console%d", id) < 0) {
+virReportOOMError();
+goto cleanup;
+}
+}
+}
+
 ret = 0;
 
 cleanup:
-- 
1.8.1.5

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[Michal Novotny]

On 06/28/2013 12:28 PM, Peter Krempa wrote:
> On 06/27/13 14:31, David Maciejak wrote:
>> Hi,
>>
>> I was discussing with Daniel about the best way to pass the ssh password
>> when using such kind of uri:
>> 'xen+libssh2://root@192.168.0.10?sshauth=password
>> '
>>
>> As it seems passing the password in the uri is not a good option, maybe we
>> can grab it from auth conf ? it seems it's not the case as now (tell me
>> if i am wrong).
> I was planing on doing this stuff, but never managed to finish this.
>
>> So enclosed a patch to add this feature.
>>
>> As you can see in virnetclient.c there is no virAuthGetPassword call, so
>> the authfile is never used.
>>
>> The patch enclosed is modifying the function prototype to add
>> virConnectPtr parameter (so remote_driver.c virNetClientNewLibSSH2 call
>> has to be updated too and the corresponding .h too).
>>
>> Once we have access to virConnectPtr, as you will see in the patch we
>> can check if authMethods is set to 'password' and grab the password from
>> auth file by calling virAuthGetPassword.
>>
>>
> please use git format-patch and send-email in the future, it makes 
> reviewing easier.
>
> See the attached patch for the review.
>
> Peter
>
>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Hi all,
Peter, you could have reformatted it for better replies on separate code
hunks ;-) Like I did now ;-)

Comment inline ...

>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>> index 7a0c1f6..012d6d5 100644
>> --- a/src/remote/remote_driver.c
>> +++ b/src/remote/remote_driver.c
>> @@ -658,7 +658,9 @@ doRemoteOpen(virConnectPtr conn,
>>sshauth,
>>netcat,
>>sockname,
>> -  auth);
>> +  auth,
>> +  conn);
>> 
> This line will require tweaking ... see below ...
>> 
> 
> ... the virConnectPtr should be the first argument in this case. Also the
> closing brace of the parameter list should be on the same line as the last
> argument.
> 
>> 
>> +   )
>>  {
>>  virNetSocketPtr sock = NULL;
>>  virNetClientPtr ret = NULL;
>> @@ -402,6 +405,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
>>  char *confdir = virGetUserConfigDirectory();
>>  char *knownhosts = NULL;
>>  char *privkey = NULL;
>> +char *password = NULL;
>>  
>>  /* Use default paths for known hosts an public keys if not provided */
>>  if (confdir) {
>> @@ -471,11 +475,21 @@ virNetClientPtr virNetClientNewLibSSH2(const char 
>> *host,
>>  if (!(command = virBufferContentAndReset(&buf)))
>>  goto no_memory;
>> 
>> -if (virNetSocketNewConnectLibSSH2(host, port, username, NULL, privkey,
>> +password = virAuthGetPassword(conn, authPtr, "libssh2", username, host);
>> +
>> +if ((strcmp(authMethods,"password") == 0) && password) {
> 
> authMethods is a comma separated list of possible methods to use. With a 
> strcmp
> this will not work if the list will contain other methods too. Use strstr() 
> instead of
> strcmp. Also this condition should make the call to virAuthGetPassword 
> conditional
> otherwise we would always ask the user for the password if it isn't stored in 
> the auth
> file even if it will never be used.


You suggest using strstr() ? Interesting libvirt doesn't have it's own 
implementation like it does on many other things. However using the substring 
matching approach is not good IMHO as it can also match the substring of real 
method, e.g. if you have e.g. authMethods = "password,auth-password" (bogus but 
works for the example to demonstrate) and you do strstr(authMethods, 
"password") is can also match if authMethods = "auth-password" but doesn't 
contain real "password" method.

I suggest tokenizing it after commas, like I'm doing for my personal projects, 
e.g. something like:

typedef struct tTokenizer {
char **tokens;
int numTokens;
} tTokenizer;

tTokenizer tokenize(char *string, char *by);
void free_tokens(tTokenizer t);

tTokenizer tokenize(char *string, char *by)
{
char *tmp;
char *str;
char *save;
char *token;
int i = 0;
tTokenizer t;

tmp = strdup(string);
t.tokens = malloc( sizeof(char *) );
for (str = tmp; ; str = NULL) {
token = strtok_r(str, by, &save);
if (token == NULL)
break;

t.tokens = realloc( t.tokens, (i + 1) * sizeof(char *) );
t.tokens[i++] = strdup(token);
}

t.numTokens = i;
return t;
}

void free_tokens(tTokenizer t)
{
int i;

for (i = 0; i < t.numToken

Re: [libvirt] [PATCHv2 0/2] Swap order of AddImplicitControllers and DomainDefPostParse

[Viktor Mihajlovski]

On 06/28/2013 09:56 AM, Ján Tomko wrote:




ACK to both. Now pushed.

Jan


Thanks!

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH] build: Fix VPATH build for access/*

[Viktor Mihajlovski]

On 06/28/2013 12:46 PM, Jiri Denemark wrote:

On Thu, Jun 27, 2013 at 18:24:16 +0200, Viktor Mihajlovski wrote:

VPATH build failed for the generated access driver files.

Signed-off-by: Viktor Mihajlovski 
---
  src/Makefile.am |   23 +--
  1 file changed, 13 insertions(+), 10 deletions(-)


ACK and pushed, thanks. I had the exact same patch in my tree but
somehow forgot to push it :-(

Jirka


:-) ... no worry, I just happen to stumble over these all of the time
as I'm exclusively building in a seperate directory.

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH] build: Fix VPATH build for access/*

[Jiri Denemark]

On Thu, Jun 27, 2013 at 18:24:16 +0200, Viktor Mihajlovski wrote:
> VPATH build failed for the generated access driver files.
> 
> Signed-off-by: Viktor Mihajlovski 
> ---
>  src/Makefile.am |   23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)

ACK and pushed, thanks. I had the exact same patch in my tree but
somehow forgot to push it :-(

Jirka

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

Re: [libvirt] [PATCH]Fix vPort Manage of FC vHBA creation

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 12:28:25PM +0200, Ján Tomko wrote:
> On 06/28/2013 11:39 AM, Daniel P. Berrange wrote:
> > On Fri, Jun 28, 2013 at 05:30:14PM +0800, Dennis Chen wrote:
> >> On 06/28/2013 04:39 PM, Ján Tomko wrote:
> >>> On 06/28/2013 03:22 AM, Dennis Chen wrote:
>  When create a virtual FC HBA with virsh/libvirt API, an error message 
>  will be
>  returned:"error: Node device not found",
>  also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn for 
>  the new
>  created device.
> 
>  Signed-off-by:xsc...@tnsoft.com.cn
>  ---
>  src/util/virutil.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>  diff --git a/src/util/virutil.c b/src/util/virutil.c
>  index 6fa0212e..569d035 100644
>  --- a/src/util/virutil.c
>  +++ b/src/util/virutil.c
>  @@ -1792,8 +1792,8 @@ virManageVport(const int parent_host,
>   if (virAsprintf(&vport_name,
>   "%s:%s",
>  -wwnn,
>  -wwpn) < 0) {
>  +wwpn,
>  +wwnn) < 0) {
>   virReportOOMError();
>   goto cleanup;
>   }
> 
> >>> Hmm, this is what we've had before commit f90af69 [1]
> >>> but according to scsi_fc_transport.txt [2] in kernel docs, it should be
> >>> :. I wonder what that commit was trying to fix.
> >>>
> >>> Jan
> >>>
> >>> [1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f90af69
> >>> [2] https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt
> >>>
> >> Interesting! According to my testing result (kernel version 2.6.32),
> >> kernel docs is correct,it should be :. It's causing me
> >> trouble when creating the device with virsh after that commit...
> > 
> > I say ACK to your patch.  Your patch matches the kernel documentation
> > and you've confirmed that it fixes a clear bug.  The original patch
> > of Osiers has zero information about what it was fixing, so there's
> > little reason to assume it was a correct change based on your feedback.
> 
> I found it: it was being called with the parameters swapped in the scsi
> storage backend.

Ah that makes sense now. The node device code was calling it the
right way, so Osier's patch broke the node device code, but fixed
his storage code which was buggy.

> 
> Pushing with this squashed in:
> 
> diff --git a/src/storage/storage_backend_scsi.c
> b/src/storage/storage_backend_scsi.c
> index 285c5cb..3deceda 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -659,8 +659,8 @@ createVport(virStoragePoolSourceAdapter adapter)
>  if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
>  return -1;
> 
> -if (virManageVport(parent_host, adapter.data.fchost.wwnn,
> -   adapter.data.fchost.wwpn, VPORT_CREATE) < 0)
> +if (virManageVport(parent_host, adapter.data.fchost.wwpn,
> +   adapter.data.fchost.wwnn, VPORT_CREATE) < 0)
>  return -1;
> 
>  virFileWaitForDevices();
> @@ -682,8 +682,8 @@ deleteVport(virStoragePoolSourceAdapter adapter)
>  if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
>  return -1;
> 
> -if (virManageVport(parent_host, adapter.data.fchost.wwnn,
> -   adapter.data.fchost.wwpn, VPORT_DELETE) < 0)
> +if (virManageVport(parent_host, adapter.data.fchost.wwpn,
> +   adapter.data.fchost.wwnn, VPORT_DELETE) < 0)
>  return -1;
> 
>  return 0;
> 

ACK

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]Fix vPort Manage of FC vHBA creation

[Ján Tomko]

On 06/28/2013 11:39 AM, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 05:30:14PM +0800, Dennis Chen wrote:
>> On 06/28/2013 04:39 PM, Ján Tomko wrote:
>>> On 06/28/2013 03:22 AM, Dennis Chen wrote:
 When create a virtual FC HBA with virsh/libvirt API, an error message will 
 be
 returned:"error: Node device not found",
 also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn for the 
 new
 created device.

 Signed-off-by:xsc...@tnsoft.com.cn
 ---
 src/util/virutil.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/util/virutil.c b/src/util/virutil.c
 index 6fa0212e..569d035 100644
 --- a/src/util/virutil.c
 +++ b/src/util/virutil.c
 @@ -1792,8 +1792,8 @@ virManageVport(const int parent_host,
  if (virAsprintf(&vport_name,
  "%s:%s",
 -wwnn,
 -wwpn) < 0) {
 +wwpn,
 +wwnn) < 0) {
  virReportOOMError();
  goto cleanup;
  }

>>> Hmm, this is what we've had before commit f90af69 [1]
>>> but according to scsi_fc_transport.txt [2] in kernel docs, it should be
>>> :. I wonder what that commit was trying to fix.
>>>
>>> Jan
>>>
>>> [1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f90af69
>>> [2] https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt
>>>
>> Interesting! According to my testing result (kernel version 2.6.32),
>> kernel docs is correct,it should be :. It's causing me
>> trouble when creating the device with virsh after that commit...
> 
> I say ACK to your patch.  Your patch matches the kernel documentation
> and you've confirmed that it fixes a clear bug.  The original patch
> of Osiers has zero information about what it was fixing, so there's
> little reason to assume it was a correct change based on your feedback.

I found it: it was being called with the parameters swapped in the scsi
storage backend.

Pushing with this squashed in:

diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 285c5cb..3deceda 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -659,8 +659,8 @@ createVport(virStoragePoolSourceAdapter adapter)
 if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
 return -1;

-if (virManageVport(parent_host, adapter.data.fchost.wwnn,
-   adapter.data.fchost.wwpn, VPORT_CREATE) < 0)
+if (virManageVport(parent_host, adapter.data.fchost.wwpn,
+   adapter.data.fchost.wwnn, VPORT_CREATE) < 0)
 return -1;

 virFileWaitForDevices();
@@ -682,8 +682,8 @@ deleteVport(virStoragePoolSourceAdapter adapter)
 if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
 return -1;

-if (virManageVport(parent_host, adapter.data.fchost.wwnn,
-   adapter.data.fchost.wwpn, VPORT_DELETE) < 0)
+if (virManageVport(parent_host, adapter.data.fchost.wwpn,
+   adapter.data.fchost.wwnn, VPORT_DELETE) < 0)
 return -1;

 return 0;

Jan

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

Re: [libvirt] [PATCH] add support for libssh2 password from auth file

[Peter Krempa]

On 06/27/13 14:31, David Maciejak wrote:

Hi,

I was discussing with Daniel about the best way to pass the ssh password
when using such kind of uri:
'xen+libssh2://root@192.168.0.10?sshauth=password
'

As it seems passing the password in the uri is not a good option, maybe we
can grab it from auth conf ? it seems it's not the case as now (tell me
if i am wrong).


I was planing on doing this stuff, but never managed to finish this.



So enclosed a patch to add this feature.

As you can see in virnetclient.c there is no virAuthGetPassword call, so
the authfile is never used.

The patch enclosed is modifying the function prototype to add
virConnectPtr parameter (so remote_driver.c virNetClientNewLibSSH2 call
has to be updated too and the corresponding .h too).

Once we have access to virConnectPtr, as you will see in the patch we
can check if authMethods is set to 'password' and grab the password from
auth file by calling virAuthGetPassword.




please use git format-patch and send-email in the future, it makes 
reviewing easier.


See the attached patch for the review.

Peter

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7a0c1f6..012d6d5 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -658,7 +658,9 @@ doRemoteOpen(virConnectPtr conn,
   sshauth,
   netcat,
   sockname,
-  auth);
+  auth,
+  conn);

This line will require tweaking ... see below ...

+
 if (!priv->client)
 goto failed;
 
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index fed2c87..75c22d7 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -37,6 +37,7 @@
 #include "virutil.h"
 #include "virerror.h"
 #include "virstring.h"
+#include "virauth.h"
 
 #define VIR_FROM_THIS VIR_FROM_RPC
 
@@ -389,7 +390,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
const char *authMethods,
const char *netcatPath,
const char *socketPath,
-   virConnectAuthPtr authPtr)
+   virConnectAuthPtr authPtr,
+   virConnectPtr conn

... the virConnectPtr should be the first argument in this case. Also the
closing brace of the parameter list should be on the same line as the last
argument.


+   )
 {
 virNetSocketPtr sock = NULL;
 virNetClientPtr ret = NULL;
@@ -402,6 +405,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
 char *confdir = virGetUserConfigDirectory();
 char *knownhosts = NULL;
 char *privkey = NULL;
+char *password = NULL;
 
 /* Use default paths for known hosts an public keys if not provided */
 if (confdir) {
@@ -471,11 +475,21 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
 if (!(command = virBufferContentAndReset(&buf)))
 goto no_memory;
 
-if (virNetSocketNewConnectLibSSH2(host, port, username, NULL, privkey,
+password = virAuthGetPassword(conn, authPtr, "libssh2", username, host);
+
+if ((strcmp(authMethods,"password") == 0) && password) {

authMethods is a comma separated list of possible methods to use. With a strcmp
this will not work if the list will contain other methods too. Use strstr() instead of
strcmp. Also this condition should make the call to virAuthGetPassword conditional
otherwise we would always ask the user for the password if it isn't stored in the auth
file even if it will never be used.

Additionally, even with the password method enabled, the password authentication
may be skipped as other methods can be used before. This would query the user
for the password allways even if it wouldn't be used ... for example as agent
based authentication would succeed before password based auth would be tried.

+
+  if (virNetSocketNewConnectLibSSH2(host, port, username, password, privkey,
   knownhosts, knownHostsVerify, authMethods,
   command, authPtr, &sock) != 0)
-goto cleanup;
+  goto cleanup;
 
+} else {
+  if (virNetSocketNewConnectLibSSH2(host, port, username, NULL, privkey,
+  knownhosts, knownHostsVerify, authMethods,
+  command, authPtr, &sock) != 0)
+  goto cleanup;
+}


 if (!(ret = virNetClientNew(sock, NULL)))
 goto cleanup;
 sock = NULL;
diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h
index 4204a93..ccaf8ab 100644
--- a/src/rpc/virnetclient.h
+++ b/src/rpc/virn

[libvirt] RFC: Introduce API to query IP addresses for given domain

[Nehal J. Wani]

Hello, fellow developers!
  I am a GSoC candidate this year working for libvirt.org. My
project is "Introduce API to query IP addresses for given domain".
The discussion regarding this feature had started here:
http://www.mail-archive.com/libvir-list@redhat.com/msg51857.html and
then Michal had sent a patch too (refer:
http://www.mail-archive.com/libvir-list@redhat.com/msg57141.html). But
it was not pushed upstream due to lack of extensibility.

So far I've come up with an API and I want to get your opinion before
I start writing the rest so I don't follow the wrong direction.

Following are the valid commands:

domifaddr 
domifaddr  
domifaddr   
domifaddr  

methods:
(i) Querying qemu-guest-agent
(ii) Getting info from dnsmasq.leases file
(iii) Using the nwfilter to snoop the traffic

If no method is mentioned, qemu-guest-agent will be used.

Previous attempts by Michal had used structs and xml. Structs bring in
restrictions and xml has to be parsed. Hence I am not planning to
continue with either of these.

As a start, I would like to know your comments about my API which
queries the qemu-guest-agent and returns the results in
virTypedParameter **params.

Format(JSON) in which the qemu guest agent returns info:

[{"name":"lo",
"ip-addresses":
[{"ip-address-type":"ipv4","ip-address":"127.0.0.1","prefix":8},
{"ip-address-type":"ipv6","ip-address":"::1","prefix":128}],
"hardware-address":"00:00:00:00:00:00"},
{"name":"eth0",
"ip-addresses":

[{"ip-address-type":"ipv4","ip-address":"192.168.122.42","prefix":24},

{"ip-address-type":"ipv6","ip-address":"fe80::5054:ff:fe09:d240","prefix":64}],
"hardware-address":"52:54:00:09:d2:40"}]

//Possible 1-D Structure (A little hassle to maintain)

params[0] = {"iface-count",int,2}
params[1] = {"iface-name",string,"lo"}
params[2] = {"iface-hwaddr",string,"00:00:00:00:00:00"}
params[3] = {"iface-addr-count",int,2}
params[4] = {"iface-addr-type",string,"ipv4"}
params[5] = {"iface-addr",string,"127.0.0.1"}
params[6] = {"iface-addr-prefix",int,8}
params[7] = {"iface-addr-type",string,"ipv6"}
params[8] = {"iface-addr",string,"::1"}
params[9] = {"iface-addr-prefix",int,128}




//2D Structure: (Not very hasslefree, but easier to maintain as one
interface per row)

params[0] = 
{"iface-name",string,"lo"}{"iface-hwaddr",string,"00:00:00:00:00:00"}{"iface-addr-type",string,"ipv4"}{"iface-addr",string,"127.0.0.1"}{"iface-addr-prefix",int,8}{"iface-addr-type",string,"ipv6"}{"iface-addr",string,"::1"}{"iface-addr-prefix",int,128}
params[1] = 
{"iface-name",string,"eth0"}{"iface-hwaddr",string,"52:54:00:09:d2:40"}{"iface-addr-type",string,"ipv4"}{"iface-addr",string,"192.168.122.42"}{"iface-addr-prefix",int,8}{"iface-addr-type",string,"ipv6"}{"iface-addr",string,"fe80::5054:ff:fe09:d240"}{"iface-addr-prefix",int,64}


Function definitions that I intend to use are:

static int
remoteDispatchDomainInterfacesAddresses(
virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client,
virNetMessagePtr msg ATTRIBUTE_UNUSED,
virNetMessageErrorPtr rerr,
remote_domain_interfaces_addresses_args *args,
remote_domain_interfaces_addresses_ret *ret)

int virDomainInterfacesAddresses (virDomainPtr dom,
 virTypedParameterPtr *params,
  int *nparams,
  unsigned int flags);


typedef int
(*virDrvDomainInterfacesAddresses)   (virDomainPtr dom,
  virTypedParameterPtr *params,
  int *nparams,
  unsigned int flags);



int
virDomainInterfacesAddresses (virDomainPtr dom,
 virTypedParameterPtr *params,
 int *nparams,
 unsigned int flags)

int
qemuAgentGetInterfaces(qemuAgentPtr mon,
   virTypedParameterPtr *params,
   int *nparams)


int qemuAgentGetInterfaces(qemuAgentPtr mon,
   virTypedParameterPtr *params,
   int *nparams);


static int
qemuDomainInterfacesAddresses(virDomainPtr dom,
  virTypedParameterPtr *params,
  int *nparams,
  unsigned int flags)

static int
remoteDomainInterfacesAddresses(virDomainPtr dom,
   virTypedParameterPtr *params,
   int *nparams,
unsigned int flags)

static bool
cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)

Also, It will be helpful to know whether we want the client to first
query for the number of parameters and then send the request or have
the server side allocate appropriate memory and return the result once
in for all. In the latter case, I'll be using something of the kind
virTyped

[libvirt] Bug 979360 - Libvirt fails to Bootstrap fails for local gnulib with 1.0.6

[chandrashekar shastri]

Hi All,

I have filed the following bug for Libvirt (Upstream) Compilation with 
local gnulib issue :


*Bug 979360*  
-Libvirt fails to Bootstrap fails for local gnulib with 1.0.6


Thanks,

Chandrashekar

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

Re: [libvirt] [RFC PATCH 2/2] LXC: Create ro overlay mounts only if we're not within a user namespace

[Daniel P. Berrange]

On Thu, Jun 27, 2013 at 08:56:25AM +0800, Gao feng wrote:
> On 06/26/2013 07:01 PM, Daniel P. Berrange wrote:
> > On Wed, Jun 26, 2013 at 05:56:19PM +0800, Gao feng wrote:
> >> On 06/26/2013 05:38 PM, Daniel P. Berrange wrote:
> >>> On Wed, Jun 26, 2013 at 10:26:10AM +0800, Gao feng wrote:
>  On 06/26/2013 04:39 AM, Daniel P. Berrange wrote:
> > On Thu, Jun 13, 2013 at 08:02:18PM +0200, Richard Weinberger wrote:
> >> Within a user namespace root can remount these filesysems at any
> >> time rw.
> >> Create these mappings only if we're not playing with user namespaces.
> >
> > This is a problem with the way we're initializing mounts in the
> > user namespace. 
> 
>  This problem exists even libvirt lxc doesn't support user namespace.
> >>>
> >>> Yes, and this is a problem that user namespace is intended to
> >>> solve.
> >>>
> > We need to ensure that the initial mounts setup
> > by libvirt can't be changed by admin inside the container. Preventing
> > the container admin from remounting or unmounting these mounts is key
> > to security.
> >
> > IIUC, the only way to ensure this is to start a new user namespace
> > /after/ setting up all mounts.
> >
> 
>  start a new user namespace means the container will lose controller of
>  mount namespace. so the container can't do mount operation too, though
>  we only can mount a little of filesystems in un-init user namespace.
> >>>
> >>> Merely being able to unmount is sufficient to exploit the host. Consider
> >>> that the container was configured with the following mapping
> >>>
> >>>   / -> /
> >>>   /export/mycontainer/home -> /home
> >>>
> >>> Now, if the container admin can umount /home, then they can now
> >>> see the home directory contents of the host. At least this is
> >>> likely to be information leakage, and if any of the host home
> >>> directories have UIDs that overlap with those assigned to the
> >>> container ID map, you have a potentially exploitable situation.
> >>>
> >>> Hence we need to ensure that the container cannot unmount or
> >>> remount anything setup by libvirt. AFAICT, this means that all
> >>> the mounts libvirt does, must be performed in a seprate user
> >>> namespace to that wit hthe container will eventually run in.
> >>>
> >>
> >> Libvirt mounts something for the container in one user namesapce,
> >> and then libvirt calls unshare to create a new user namespace and
> >> start the init task of container.
> >>
> >> Yes, the users in container can't do mount/unmount/remount on all
> >> of filesystem. but they can call unshare to create a new mount namespace,
> >> and they will have rights to mount/unmount/remount in this new created
> >> mount namespace. they can still umount /home to see the home directory
> >> contents of host.
> > 
> > An existing filesystem mount can only be remounted/unmounted by the
> > (user ID, usernamespace) that originally mounted it. So even if you
> > start a new mount namespace, you cannot unmount stuff setup by the
> > parent user namespace.
> > 
> 
> Please also setup the uid_map/gid_map for the unshared user namespace.
> even in container, user has rights to setup these two files.
> 
> > # unshare --mount --user /bin/sh
> > sh-4.2$ umount /sys/kernel/debug
> > umount: /sys/kernel/debug: Invalid argument
> > 
> 
> in terminal one
> $ id
> uid=1000(gaofeng) gid=1000(gaofeng) groups=1000(gaofeng)
> $ ./unshare --mount --user /bin/sh
> sh-4.2$ echo $$
> 17110
> sh-4.2$
> 
> in other terminal,setup id map for new userns.
> $echo 0 1000 1 > /proc/17110/uid_map
> $echo 0 1000 1 > /proc/17110/gid_map
> 
> and then in terminal one
> sh-4.2$ umount -l /home/

Oh, hmm, forgot about the uid mapping. I thought the capabilities would
be allowing me unmount regardless.

Well, given that we're at rc2 now & I'm still unclear about how some
aspects of the userns setup is working, I'm afraid we'll have to wait
until 1.1.1 for the userns LXC code to merge.  I'll aim todo it next
week, so that we have plenty of time for further testing before the
1.1.1 release.

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] add support for libssh2 password from auth file

[David Maciejak]

enclosed the patch from git repo


On Thu, Jun 27, 2013 at 2:31 PM, David Maciejak wrote:

> Hi,
>
> I was discussing with Daniel about the best way to pass the ssh password
> when using such kind of uri: 'xen+libssh2://
> root@192.168.0.10?sshauth=password'
>
> As it seems passing the password in the uri is not a good option, maybe we
> can grab it from auth conf ? it seems it's not the case as now (tell me if
> i am wrong).
>
> So enclosed a patch to add this feature.
>
> As you can see in virnetclient.c there is no virAuthGetPassword call, so
> the authfile is never used.
>
> The patch enclosed is modifying the function prototype to add
> virConnectPtr parameter (so remote_driver.c virNetClientNewLibSSH2 call has
> to be updated too and the corresponding .h too).
>
> Once we have access to virConnectPtr, as you will see in the patch we
> can check if authMethods is set to 'password' and grab the password from
> auth file by calling virAuthGetPassword.
>
>
>
> regards,
> david
>


virnetclient.patch
Description: Binary data
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH]Fix vPort Manage of FC vHBA creation

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 05:30:14PM +0800, Dennis Chen wrote:
> On 06/28/2013 04:39 PM, Ján Tomko wrote:
> >On 06/28/2013 03:22 AM, Dennis Chen wrote:
> >>When create a virtual FC HBA with virsh/libvirt API, an error message will 
> >>be
> >>returned:"error: Node device not found",
> >>also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn for the 
> >>new
> >>created device.
> >>
> >>Signed-off-by:xsc...@tnsoft.com.cn
> >>---
> >>src/util/virutil.c |4 ++--
> >>1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/src/util/virutil.c b/src/util/virutil.c
> >>index 6fa0212e..569d035 100644
> >>--- a/src/util/virutil.c
> >>+++ b/src/util/virutil.c
> >>@@ -1792,8 +1792,8 @@ virManageVport(const int parent_host,
> >>  if (virAsprintf(&vport_name,
> >>  "%s:%s",
> >>-wwnn,
> >>-wwpn) < 0) {
> >>+wwpn,
> >>+wwnn) < 0) {
> >>  virReportOOMError();
> >>  goto cleanup;
> >>  }
> >>
> >Hmm, this is what we've had before commit f90af69 [1]
> >but according to scsi_fc_transport.txt [2] in kernel docs, it should be
> >:. I wonder what that commit was trying to fix.
> >
> >Jan
> >
> >[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f90af69
> >[2] https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt
> >
> Interesting! According to my testing result (kernel version 2.6.32),
> kernel docs is correct,it should be :. It's causing me
> trouble when creating the device with virsh after that commit...

I say ACK to your patch.  Your patch matches the kernel documentation
and you've confirmed that it fixes a clear bug.  The original patch
of Osiers has zero information about what it was fixing, so there's
little reason to assume it was a correct change based on your feedback.

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] Bug 979260 - cpu hot-add doesn't work with upstream libvirt 1.0.6 + qemu 1.5.50

[Peter Krempa]

On 06/28/13 11:25, chandrashekar shastri wrote:

Hi All,

I have filed the following bug for CPU hot-add issue:

Bug 979260 - cpu hot-add doesn't work with upstream libvirt 1.0.6 + qemu
1.5.50


The hotplug feature in qemu 1.5 uses a new monitor command. The support 
was recently added to libvirt by:


commit c12b2be5169298708cf727ed9ccd42e9d89a9737
Author: Peter Krempa 
Date:   Mon May 27 16:08:30 2013 +0200

qemu: Implement new QMP command for cpu hotplug

This patch implements support for the "cpu-add" QMP command that plugs
CPUs into a live guest. The "cpu-add" command was introduced in QEMU
1.5. For the hotplug to work machine type "pc-i440fx-1.5" is required.

The commit will be included in the upcomming 1.1.0 release of libvirt.

Peter

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

Re: [libvirt] [PATCH]Fix vPort Manage of FC vHBA creation

[Dennis Chen]

On 06/28/2013 04:39 PM, Ján Tomko wrote:

On 06/28/2013 03:22 AM, Dennis Chen wrote:

When create a virtual FC HBA with virsh/libvirt API, an error message will be
returned:"error: Node device not found",
also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn for the new
created device.

Signed-off-by:xsc...@tnsoft.com.cn
---
src/util/virutil.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 6fa0212e..569d035 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1792,8 +1792,8 @@ virManageVport(const int parent_host,
  
  if (virAsprintf(&vport_name,

  "%s:%s",
-wwnn,
-wwpn) < 0) {
+wwpn,
+wwnn) < 0) {
  virReportOOMError();
  goto cleanup;
  }


Hmm, this is what we've had before commit f90af69 [1]
but according to scsi_fc_transport.txt [2] in kernel docs, it should be
:. I wonder what that commit was trying to fix.

Jan

[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f90af69
[2] https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt

Interesting! According to my testing result (kernel version 2.6.32), 
kernel docs is correct,it should be :. It's causing me 
trouble when creating the device with virsh after that commit...


BRs,
Dennis

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

[libvirt] Bug 979260 - cpu hot-add doesn't work with upstream libvirt 1.0.6 + qemu 1.5.50

[chandrashekar shastri]

Hi All,

I have filed the following bug for CPU hot-add issue:

Bug 979260 - cpu hot-add doesn't work with upstream libvirt 1.0.6 + qemu 
1.5.50


Thanks,
Chandrashekar

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

Re: [libvirt] [PATCH V2] Expose all CPU features in host definition

[Daniel P. Berrange]

On Thu, Jun 27, 2013 at 10:35:58AM -0600, Don Dugger wrote:
> On Mon, Jun 17, 2013 at 10:27:36AM +0200, Jiri Denemark wrote:
> > On Fri, Jun 14, 2013 at 12:32:35 -0600, Don Dugger wrote:
> > > On Fri, Jun 14, 2013 at 03:06:52PM +0200, Jiri Denemark wrote:
> > > > I was just trying to say that it doesn't provide anything more than
> > > > "it's supported by the host CPU", which gives mostly no value in the
> > > > context of libvirt. Can you explain more what the use case is in which a
> > > > virt client would need to know what specific feature are supported by
> > > > host CPU? I feel like we should avoid people from being under the
> > > > impression that they can actually use the CPU capabilities for checking
> > > > whether a host can run guests that require specific CPU features.
> > > 
> > > The specific use case I'm trying to address is a cloud environment where,
> > > with hundreds of hosts, you might want to schedule an instance to a host
> > > that supports a particular HW acceleration, like AES/NI.  I agree, what
> > > I `really` want is an API that shows the capabilities of a specific guest
> > > that could be created on the host but, absent that API, at least knowing
> > > that a host doesn't support the feature is more information than I can get
> > > right now.
> > 
> > Hmm, fair enough. Let's wait a few days for Daniel to return from
> > vacation in case he wants to express his opinion here.
> 
> So, any progress here?

I believe the cloud use case above is approaching the problem in the wrong
way. We designed our APIs such that apps should never need to write logic
for comparing CPU features directly. If an application has a set of CPU
features it requires from the host, then it should use a libvirt API to
query whether those features are available, not try to write the CPU
fetaure comparison logic itself.

You can already pretty much do this with te virConnectCompareCPU()
method, by passing an XML document which specifies the AES/NI feature
flag that you want to check for support of. Then libvirt will tell
you whether the host CPU can support it. It is entirely possible to
make use of this capability as is in OpenStack.

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 v2] Make logical pools independent on target path

[Martin Kletzander]

When using logical pools, we had to trust the target->path provided.
This parameter, however, can be completely ommited and we can get the
correct path using 'lvdisplay' command.  In order not to omit the
target.path completely, we rather default it to '/dev/'
which is used to check the pool anyway.

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

Signed-off-by: Martin Kletzander 
---

Notes:
v2:
 - don't drop target.path, but fix it instead to '/dev/'

Thanks to the change from v1, we can now safely drop all the hunks
from logical backend and even the dependency on lvdisplay command.
There might be some systems where the path is different and the part
of this patch using lvdisplay command would make most of them work.
However, checkPool() still depends on '/dev/' and I
haven't found any other way how to fix that, so feel free to ACK just
the {conf,docs,tests}/ part in case you think that we shouldn't try to
support anything else than '/dev/'.

 configure.ac   |  4 +
 docs/schemas/storagepool.rng   | 13 ++-
 src/conf/storage_conf.c| 19 +++--
 src/storage/storage_backend_logical.c  | 95 +-
 src/storage/storage_driver.c   |  2 +-
 tests/storagepoolxml2xmlin/pool-logical-create.xml |  2 +-
 tests/storagepoolxml2xmlin/pool-logical-nopath.xml | 18 
 .../storagepoolxml2xmlout/pool-logical-nopath.xml  | 19 +
 tests/storagepoolxml2xmltest.c |  1 +
 9 files changed, 125 insertions(+), 48 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-logical-nopath.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-logical-nopath.xml

diff --git a/configure.ac b/configure.ac
index ef246a6..221b6cb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1601,6 +1601,7 @@ if test "$with_storage_lvm" = "yes" || test 
"$with_storage_lvm" = "check"; then
   AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin])
   AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin])
   AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin])
+  AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin])

   if test "$with_storage_lvm" = "yes" ; then
 if test -z "$PVCREATE" ; then AC_MSG_ERROR([We need pvcreate for LVM 
storage driver]) ; fi
@@ -1615,6 +1616,7 @@ if test "$with_storage_lvm" = "yes" || test 
"$with_storage_lvm" = "check"; then
 if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage 
driver]) ; fi
 if test -z "$VGS" ; then AC_MSG_ERROR([We need vgs for LVM storage 
driver]) ; fi
 if test -z "$LVS" ; then AC_MSG_ERROR([We need lvs for LVM storage 
driver]) ; fi
+if test -z "$LVDISPLAY" ; then AC_MSG_ERROR([We need lvdisplay for LVM 
storage driver]) ; fi
   else
 if test -z "$PVCREATE" ; then with_storage_lvm=no ; fi
 if test -z "$VGCREATE" ; then with_storage_lvm=no ; fi
@@ -1628,6 +1630,7 @@ if test "$with_storage_lvm" = "yes" || test 
"$with_storage_lvm" = "check"; then
 if test -z "$PVS" ; then with_storage_lvm=no ; fi
 if test -z "$VGS" ; then with_storage_lvm=no ; fi
 if test -z "$LVS" ; then with_storage_lvm=no ; fi
+if test -z "$LVDISPLAY" ; then with_storage_lvm=no ; fi

 if test "$with_storage_lvm" = "check" ; then with_storage_lvm=yes ; fi
   fi
@@ -1646,6 +1649,7 @@ if test "$with_storage_lvm" = "yes" || test 
"$with_storage_lvm" = "check"; then
 AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program])
 AC_DEFINE_UNQUOTED([VGS],["$VGS"],[Location of vgs program])
 AC_DEFINE_UNQUOTED([LVS],["$LVS"],[Location of lvs program])
+AC_DEFINE_UNQUOTED([LVDISPLAY],["$LVDISPLAY"],[Location of lvdisplay 
program])
   fi
 fi
 AM_CONDITIONAL([WITH_STORAGE_LVM], [test "$with_storage_lvm" = "yes"])
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 3c2158a..1b3f4bc 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -62,7 +62,7 @@
 
 
 
-
+
   

   
@@ -207,6 +207,17 @@
 
   

+  
+
+  
+
+  
+
+  
+  
+
+  
+
   
 
   
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 288e265..7ffe58c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1,7 +1,7 @@
 /*
  * storage_conf.c: config handling for storage driver
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -959,15 +959,22 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
 /* When we are working with a virtual disk we can skip the target
  * path and permissions */
 if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) {
-if (!(target_path = virXPathString("string(./target/path)", ctxt))) {
-virReportError(VIR_E

Re: [libvirt] [PATCH] Drop iptablesContext

[Daniel P. Berrange]

On Fri, Jun 28, 2013 at 12:52:30AM -0400, Roman Bogorodskiy wrote:
> iptablesContext holds only 4 pairs of iptables
> (table, chain) and there's no need to pass
> it around.
> 
> This is a first step towards separating bridge_driver.c
> in platform-specific parts.
> ---
>  src/libvirt_private.syms|   2 -
>  src/network/bridge_driver.c | 253 +--
>  src/util/viriptables.c  | 257 
> +++-
>  src/util/viriptables.h  |  65 ---
>  4 files changed, 183 insertions(+), 394 deletions(-)

ACK

Thanks for doing this - it has been on my death-list for a quite
a while now :-)


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]Fix vPort Manage of FC vHBA creation

[Ján Tomko]

On 06/28/2013 03:22 AM, Dennis Chen wrote:
> When create a virtual FC HBA with virsh/libvirt API, an error message will be
> returned:"error: Node device not found",
> also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn for the new
> created device.
> 
> Signed-off-by:xsc...@tnsoft.com.cn
> ---
> src/util/virutil.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 6fa0212e..569d035 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1792,8 +1792,8 @@ virManageVport(const int parent_host,
>  
>  if (virAsprintf(&vport_name,
>  "%s:%s",
> -wwnn,
> -wwpn) < 0) {
> +wwpn,
> +wwnn) < 0) {
>  virReportOOMError();
>  goto cleanup;
>  }
> 

Hmm, this is what we've had before commit f90af69 [1]
but according to scsi_fc_transport.txt [2] in kernel docs, it should be
:. I wonder what that commit was trying to fix.

Jan

[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f90af69
[2] https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt

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

Re: [libvirt] [PATCH] Drop iptablesContext

[Laine Stump]

On 06/28/2013 12:52 AM, Roman Bogorodskiy wrote:
> iptablesContext holds only 4 pairs of iptables
> (table, chain) and there's no need to pass
> it around.
>
> This is a first step towards separating bridge_driver.c
> in platform-specific parts.
> ---
>  src/libvirt_private.syms|   2 -
>  src/network/bridge_driver.c | 253 +--
>  src/util/viriptables.c  | 257 
> +++-
>  src/util/viriptables.h  |  65 ---
>  4 files changed, 183 insertions(+), 394 deletions(-)

Mostly mechanical. Visual/build ACK. I'll run some tests tomorrow, and
push it after 1.1.0 is released.


(It's just amazing how much change was required just to remove that one
thing.

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

Re: [libvirt] Candidate release 2 for libvirt-1.1.0

[Laine Stump]

On 06/28/2013 03:44 AM, Laine Stump wrote:
> On 06/28/2013 03:24 AM, Jason Helfman wrote:
>>
>> On Thu, Jun 27, 2013 at 11:01 AM, Daniel Veillard
>> mailto:veill...@redhat.com>> wrote:
>>
>>I have just tagged the release candidate 2 in git and sent
>> a tarball
>> to the usual place (rpms are coming):
>>ftp://libvirt.org/libvirt/
>>
>> This includes the end of the patch set from Laine, and hopefully
>> it won't require too many other patches. I tried it and it
>> doesn't
>> look obviously broken to me, please give it a try too, especially
>> for portability :-)
>> If all goes well the final release should be next Monday !
>>
>>   thanks !
>>
>> Daniel
>>
>>
>> So far, I am getting linker errors for FreeBSD here:
>>  https://redports.org/buildarchive/20130628070800-42595/
>>
>
> Sigh. I see the problem. Patch coming up...


Okay, I pushed the following patch:

https://www.redhat.com/archives/libvir-list/2013-June/msg01171.html

Can you apply this patch locally to the source tar and re-run your test
build to make sure nothing else is broken (since there won't be another
rc before release)? If there are still problems, you can find us in
#virt on irc.oftc.net.



commit a757822233f707c4ed75986f5903e26e40f3cdfa
Author: Laine Stump 
Date:   Fri Jun 28 04:00:54 2013 -0400

util: fix build error on non-Linux systems
   
Building on FreeBSD had this linker error:
   
/work/a/ports/devel/libvirt/work/libvirt-1.1.0/src/.libs/libvirt.so:
   undefined reference to `virPCIDeviceAddressParse'
   
This was caused by the new use of virPCIDeviceAddressParse in a
portion of virpci.c that wasn't linux-only (in commit 72c029d8). The
problem was that virPCIDeviceAddressParse had originally been defined
inside #ifdef _linux (because it was only used by another function
that was inside the same ifdef).
   
The solution is to move it out to the part of virpci.c that is
compiled on all platforms.
   
(Because the portion that was "moved" was 40-50 lines, but only moved
up by 15 lines, the diff for the patch is less than non-informative -
rather than showing that part that I moved, it shows the bit that was
previously before the moved part, and now sits *after* it.)


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

[libvirt] [PATCH] util: fix build error on non-Linux systems

[Laine Stump]

Building on FreeBSD had this linker error:

/work/a/ports/devel/libvirt/work/libvirt-1.1.0/src/.libs/libvirt.so:
   undefined reference to `virPCIDeviceAddressParse'

This was caused by the new use of virPCIDeviceAddressParse in a
portion of virpci.c that wasn't linux-only (in commit 72c029d8). The
problem was that virPCIDeviceAddressParse had originally been defined
inside #ifdef _linux (because it was only used by another function
that was inside the same ifdef).

The solution is to move it out to the part of virpci.c that is
compiled on all platforms.

(Because the portion that was "moved" was 40-50 lines, but only moved
up by 15 lines, the diff for the patch is less than non-informative -
rather than showing that part that I moved, it shows the bit that was
previously before the moved part, and now sits *after* it.)
---
Pushed under the build-breaker rule.

I actually hadn't noticed that some of the functions in virpci.c were
Linux-only. It looks like this was done because those functions use
the Linux sysfs to get information about the devices. However, much of
the rest of virpci.c does the same thing, so we should probably
revisit which of the functions in that file are compiled only on Linux
and which are compile for everyone.

 src/util/virpci.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0ed29e7..7d83bdb 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2261,21 +2261,6 @@ int virPCIDeviceIsAssignable(virPCIDevicePtr dev,
 return 1;
 }
 
-#ifdef __linux__
-
-/*
- * returns true if equal
- */
-static bool
-virPCIDeviceAddressIsEqual(virPCIDeviceAddressPtr bdf1,
-   virPCIDeviceAddressPtr bdf2)
-{
-return ((bdf1->domain == bdf2->domain) &&
-(bdf1->bus == bdf2->bus) &&
-(bdf1->slot == bdf2->slot) &&
-(bdf1->function == bdf2->function));
-}
-
 static int
 logStrToLong_ui(char const *s,
 char **end_ptr,
@@ -2327,6 +2312,21 @@ out:
 return ret;
 }
 
+#ifdef __linux__
+
+/*
+ * returns true if equal
+ */
+static bool
+virPCIDeviceAddressIsEqual(virPCIDeviceAddressPtr bdf1,
+   virPCIDeviceAddressPtr bdf2)
+{
+return ((bdf1->domain == bdf2->domain) &&
+(bdf1->bus == bdf2->bus) &&
+(bdf1->slot == bdf2->slot) &&
+(bdf1->function == bdf2->function));
+}
+
 static int
 virPCIGetDeviceAddressFromSysfsLink(const char *device_link,
 virPCIDeviceAddressPtr *bdf)
-- 
1.7.11.7

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

[libvirt] Design of virDomainSnapshotExport/Import

[Xu Wang]

Applications based on libvirt such as OpenStack need some public APIs
about export/import of external snapshot during daily backup, snapshot
recovery or system migration. virDomainSnapshotDiskExport,
virDomainSnapshotVmstateExport and virDomainSnapshotImport are designed
to satisfy such requirement.



@flags: virDomainSnapshotExportImportFlags include,
DATA_FULL: all data snapshot
DATA_DELTA: delta data only



/**
 * virDomainSnapshotDiskExport
 * @domain: the domain object to be exported snapshot
 * @id: top Overlays which to be exported
 * @base_id: RootBase of snapshot to be exported
 * @disk: block device of domain
 * @export_path: path of snapshots export
 * @flags: bitwise-OR of virDomainSnapshotExportImportFlags
 *
 * Import snapshot disk data to the pointed path.
 *
 * If @flags includes DATA_FULL, full backup snapshot will be exported.
 *
 * If @flags includes DATA_DELTA, incremental backup snapshot will be
 * exported.
 *
 * DATA_FULL and DATA_DELTA should not appear at the same time
 *
 * Returns 0 on success, or -1 on error
 */
int virDomainSnapshotDiskExport(virDomain *domain,
virSnapshotId *id,
virSnapshotId *base_id,
virDevice *disk,
char *export_path,
unsigned int flags)

Usage: export external snapshot disk file from virtual machine to the
   target path.



/**
 * virDomainSnapshotVmstateExport
 * @domain: the domain object to be exported snapshot
 * @id: top Overlays which to be exported
 * @export_path: path of snapshots export
 * @flags: not used yet, so callers should always pass 0
 *
 * Import snapshot vmstate data to the pointed path.
 *
 * If @flags includes DATA_FULL, full backup snapshot will be exported.
 *
 * If @flags includes DATA_DELTA, incremental backup snapshot will be
 * exported.
 *
 * DATA_FULL and DATA_DELTA should not appear at the same time
 *
 * Returns 0 on success, or -1 on error
 */
int virDomainSnapshotVmstateExport(virDomain *domain,
   virSnapshotId *id,
   char *export_path,
   unsigned int flags)

Usage: dump vmstate file of snapshot and copy it to the target path.



/**
 * virDomainSnapshotImport
 * @domain: the domain object to be exported snapshot
 * @xmlDesc: string containing an XML description of the domain
 * @flags: bitwise-OR of virDomainSnapshotExportImportFlags
 *
 * Import snapshot data to the pointed path.
 *
 * If @flags includes DATA_FULL, full backup snapshot will be exported.
 *
 * If @flags includes DATA_DELTA, incremental backup snapshot will be
 * exported.
 *
 * DATA_FULL and DATA_DELTA should not appear at the same time
 *
 * Returns 0 on success, or -1 on error
 */
int virDomainSnapshotImport(virDomain *domain,
const char *xmlDesc,
unsigned int flags)

Usage: import .xml file of external snapshot and use it for recovering
   a virtual machine.



Usecase:
  If users want to finish daily backup&recovery work by above APIs,
the process should be,
  1. call virDomainSnapshotDiskExport() to export pointed snapshot
disk files to the traget path.

  --- -
  |host A   | |host B |
  | --- | | - |
  | | dom1| |   snapshot disk files   | | backup folder | |
  | | base.img--->base.img  | |
  | | snap1.qcow2>snap1.qcow2   | |
  | | snap2.qcow2>snap2.qcow2   | |
  | --- | | - |
  --- -
  virDomainSnapshotDiskSnapshot()

  2. call virDomainSnapshotVmstateExport() to export vmstate data
file to the target path.

  --- -
  |   host A| |host B |
  | --- | | - |
  | | dom1| |   vmstate data files| | backup folder | |
  | | dumpxml>vmstate.xml   | |
  | --- | | - |
  --- -
 virDomainSnapshotVmstateSnapshot()

  3. when some error occured, or guest copy/migration, users copy
snapshot disk files from host B to the target host and edit  field of  tag. Then users call virDomainSnapshotImport
to recovery domain to the snapshot.

  a. copy snapshot disk files to the target host:
  --- -
  |host A   |  snapshot   |host B |
  | --- |   

Re: [libvirt] [PATCHv2 0/2] Swap order of AddImplicitControllers and DomainDefPostParse

[Ján Tomko]

On 06/17/2013 04:17 PM, Viktor Mihajlovski wrote:
> Implicit controllers may be dependent on device definitions altered
> in a post-parse callback. E.g., if a console device is
> defined without the target type, the type will be set in QEMU's
> callback. In the case of s390, this is virtio, which requires
> an implicit virtio-serial controller.
> 
> By moving the implicit controller definition after the post-parse
> procssing this can be fixed. As Martin pointed out, implicit controllers
> should not need post-parsing, so the rearranging should not hurt.
> Probably this is only affecting the S390 virtio console anyway.
> 
> V2 Changes:
>  - Promoted from RFC to Patch Series
>  - Added an qemuxml2xml testcase highlighting the issue: applying the first
>patch only will fail make check as the implicit controller is missing.
> 
> Viktor Mihajlovski (2):
>   S390: Testcase for console default target type (virtio)
>   conf: Swap order of AddImplicitControllers and DomainDefPostParse
> 
>  src/conf/domain_conf.c |8 +++
>  .../qemuxml2argv-s390-defaultconsole.xml   |   20 
>  .../qemuxml2xmlout-balloon-device-auto.xml |2 +-
>  .../qemuxml2xmlout-channel-virtio-auto.xml |2 +-
>  .../qemuxml2xmlout-console-virtio.xml  |2 +-
>  .../qemuxml2xmlout-disk-scsi-device-auto.xml   |2 +-
>  .../qemuxml2xmlout-s390-defaultconsole.xml |   24 
> 
>  tests/qemuxml2xmltest.c|2 ++
>  8 files changed, 54 insertions(+), 8 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml
> 

ACK to both. Now pushed.

Jan

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

Re: [libvirt] [PATCH] Allow RO connections to interface udev backend

[Ján Tomko]

On 06/28/2013 03:48 AM, Doug Goldstein wrote:
> The udev based interface backend did not allow querying data over a
> read-only connection which is different than how the netcf backend
> operates. This brings the behavior inline with the default, netcf
> backend.
> ---
>  src/interface/interface_backend_udev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

ACK

Jan

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

Re: [libvirt] Candidate release 2 for libvirt-1.1.0

[Laine Stump]

On 06/28/2013 03:24 AM, Jason Helfman wrote:
>
> On Thu, Jun 27, 2013 at 11:01 AM, Daniel Veillard
> mailto:veill...@redhat.com>> wrote:
>
>I have just tagged the release candidate 2 in git and sent
> a tarball
> to the usual place (rpms are coming):
>ftp://libvirt.org/libvirt/
>
> This includes the end of the patch set from Laine, and hopefully
> it won't require too many other patches. I tried it and it doesn't
> look obviously broken to me, please give it a try too, especially
> for portability :-)
> If all goes well the final release should be next Monday !
>
>   thanks !
>
> Daniel
>
>
> So far, I am getting linker errors for FreeBSD here:
>  https://redports.org/buildarchive/20130628070800-42595/
>

Sigh. I see the problem. Patch coming up...
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Candidate release 2 for libvirt-1.1.0

[Jason Helfman]

> On Thu, Jun 27, 2013 at 11:01 AM, Daniel Veillard wrote:
>
>>I have just tagged the release candidate 2 in git and sent a tarball
>> to the usual place (rpms are coming):
>>ftp://libvirt.org/libvirt/
>>
>> This includes the end of the patch set from Laine, and hopefully
>> it won't require too many other patches. I tried it and it doesn't
>> look obviously broken to me, please give it a try too, especially
>> for portability :-)
>> If all goes well the final release should be next Monday !
>>
>>   thanks !
>>
>> Daniel
>>
>>
So far, I am getting linker errors for FreeBSD here:
 https://redports.org/buildarchive/20130628070800-42595/

-jgh

--
Jason Helfman  | FreeBSD Committer
j...@freebsd.org | http://people.freebsd.org/~jgh  | The Power to Serve
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list