[libvirt] [PATCH] examples: Add missing build data for 'rename'

2015-09-07 Thread Martin Kletzander 
Commit e755186c5c30 added the rename example, but forgot to build some
essential files in there as well as add it to the spec file.

Signed-off-by: Martin Kletzander 
---
Pushed under the build-breaker rule.

 Makefile.am | 2 +-
 libvirt.spec.in | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index d338d5a220c1..6f217bc08595 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs 
gnulib/tests \
   examples/dominfo examples/domsuspend examples/apparmor \
   examples/xml/nwfilter examples/openauth examples/systemtap \
   tools/wireshark examples/dommigrate examples/polkit \
-  examples/lxcconvert examples/domtop
+  examples/lxcconvert examples/domtop examples/rename

 ACLOCAL_AMFLAGS = -I m4

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6f6b191ee61d..bb8bfc3c25c1 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1546,7 +1546,7 @@ rm -fr %{buildroot}
 # on RHEL 5, thus we need to expand it here.
 make install DESTDIR=%{?buildroot} SYSTEMD_UNIT_DIR=%{_unitdir}

-for i in object-events dominfo domsuspend hellolibvirt openauth xml/nwfilter 
systemtap dommigrate domtop
+for i in object-events dominfo domsuspend hellolibvirt openauth xml/nwfilter 
systemtap dommigrate domtop rename
 do
   (cd examples/$i ; make clean ; rm -rf .deps .libs Makefile Makefile.in)
 done
@@ -2330,6 +2330,7 @@ exit 0
 %doc examples/dommigrate
 %doc examples/openauth
 %doc examples/xml
+%doc examples/rename
 %doc examples/systemtap

 %changelog
--
2.5.1

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

[libvirt] [PATCH] .gitignore: Ignore the correct rename example binary

2015-09-07 Thread Michal Privoznik 
In e755186c5c305a9 we tried to introduce an example demonstrating
new virDomainRename API. Unfortunately, in the .gitignore we had
a different binary listed. It's 'rename' binary which we want git
to ignore, not 'test'.

Signed-off-by: Michal Privoznik 
---

Pushed under trivial rule.

 .gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 19402f5..2d52a8f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,7 +83,7 @@
 /examples/domtop/domtop
 /examples/hellolibvirt/hellolibvirt
 /examples/openauth/openauth
-/examples/rename/test
+/examples/rename/rename
 /gnulib/lib/*
 /gnulib/m4/*
 /gnulib/tests/*
-- 
2.4.6

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

Re: [libvirt] [PATCH 2/3] vmx: The virVMXParseDisk deviceType can be NULL, add some missing checks

2015-09-07 Thread Michal Privoznik 
On 05.09.2015 16:29, Matthias Bolte wrote:
> 2015-09-02 13:00 GMT+02:00 Michal Privoznik :
>> On 01.09.2015 16:52, Matthias Bolte wrote:
>>> ---
>>>  src/vmx/vmx.c | 22 +++---
>>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>>> index ba4d046..9d1574f 100644
>>> --- a/src/vmx/vmx.c
>>> +++ b/src/vmx/vmx.c
>>> @@ -2178,8 +2178,9 @@ virVMXParseDisk(virVMXContext *ctx, 
>>> virDomainXMLOptionPtr xmlopt, virConfPtr con
>>>  (*def)->transient = STRCASEEQ(mode,
>>>"independent-nonpersistent");
>>>  } else if (virFileHasSuffix(fileName, ".iso") ||
>>> -   STRCASEEQ(deviceType, "atapi-cdrom") ||
>>> -   STRCASEEQ(deviceType, "cdrom-raw")) {
>>> +   (deviceType &&
>>> +(STRCASEEQ(deviceType, "atapi-cdrom") ||
>>> + STRCASEEQ(deviceType, "cdrom-raw" {
>>>  /*
>>>   * This function was called in order to parse a harddisk 
>>> device,
>>>   * but .iso files, 'atapi-cdrom', and 'cdrom-raw' devices are 
>>> for
>>> @@ -2199,13 +2200,12 @@ virVMXParseDisk(virVMXContext *ctx, 
>>> virDomainXMLOptionPtr xmlopt, virConfPtr con
>>>  if (virFileHasSuffix(fileName, ".iso")) {
>>>  char *tmp;
>>>
>>> -if (deviceType != NULL) {
>>> -if (STRCASENEQ(deviceType, "cdrom-image")) {
>>> -virReportError(VIR_ERR_INTERNAL_ERROR,
>>> -   _("Expecting VMX entry '%s' to be 
>>> 'cdrom-image' "
>>> - "but found '%s'"), deviceType_name, 
>>> deviceType);
>>> -goto cleanup;
>>> -}
>>> +if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>>> +   _("Expecting VMX entry '%s' to be 
>>> 'cdrom-image' "
>>> + "but found '%s'"), deviceType_name,
>>> +   deviceType ? deviceType : "unknown");
>>
>> The control can get here iff deviceType != NULL.
>>
> 
> The check for NULL was preexisting here, I just merged two nested if
> blocks here. Also I don't see how you get to the conclusion that the
> control can only get here if deviceType != NULL. There is no code
> before or around this block that would do that.
> 

Maybe I should have been more specific. I meant the virReportError().
There's no need for (preexisting) ternary operator since the if() right
above checks for deviceType.

Michal

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

Re: [libvirt] [PATCH v2 1/2] virsh: Slightly rework cmdDomblklist

2015-09-07 Thread Martin Kletzander 

On Wed, Sep 02, 2015 at 05:58:18PM +0200, Michal Privoznik wrote:

Let's move some variables from an inside loop to global function
declaration header block. It's going to be easier for next
patches. At the same time, order the cleanup calls at the
function's end so it's easier to track which variables are freed
and which not.

Signed-off-by: Michal Privoznik 
---
tools/virsh-domain-monitor.c | 29 ++---
1 file changed, 14 insertions(+), 15 deletions(-)



I did not follow the discussion around 2/2, but this one does not do
any functional change and cleans up the code nicely, so ACK ... [1]


diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 340a8e2..d4e500b 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -549,24 +544,28 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
if (details) {
vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device,
 target, source ? source : "-");
-VIR_FREE(type);
-VIR_FREE(device);
} else {
vshPrint(ctl, "%-10s %s\n", target, source ? source : "-");
}

-VIR_FREE(target);
VIR_FREE(source);
+VIR_FREE(target);
+VIR_FREE(device);
+VIR_FREE(type);
}

ret = true;

 cleanup:
+VIR_FREE(source);
+VIR_FREE(target);
+VIR_FREE(device);
+VIR_FREE(type);
VIR_FREE(disks);
-virDomainFree(dom);
-VIR_FREE(xml);
-xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xmldoc);
+VIR_FREE(xml);
+virDomainFree(dom);


[1] ... even though I don't understand why these are reordered.


return ret;
}

--
2.4.6

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


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

Re: [libvirt] [PATCH libvirt-java] Require at least Java 8

2015-09-07 Thread Wido den Hollander 


On 04-09-15 07:12, Claudio Bley wrote:
> At Wed, 2 Sep 2015 17:38:03 +0200,
> Wido den Hollander wrote:
>>
>>
>>
>>> The current code doesn't compile under Java 7, but Java 7 is also EOL.
>>>
>>>
>>> That should not be the case. Can you provide the error messages or
>>> simply point to the place where Java 8 classes are used?
>>>
>>
>> build:
>> [javac] Compiling 98 source files to
>> /home/wido/repos/libvirt-java/target/classes
>> [javac] warning: [options] bootstrap class path not set in
>> conjunction with -source 1.7
>> [javac]
>> /home/wido/repos/libvirt-java/src/main/java/org/libvirt/event/DomainEvent.java:63:
>> error: incompatible types: inference variable T#1 has incompatible upper
>> bounds Enum,T#3
>> [javac] return this.type.obtain(this.detail);
>> [javac]^
>> [javac]   where T#1,T#2,T#3 are type-variables:
>> [javac] T#1 extends Enum declared in method obtain(int)
>> [javac] T#2 extends T#3
>> [javac] T#3 extends Enum,DomainEventDetail declared in
>> method getDetail()
>> [javac] 1 error
>> [javac] 1 warning
> 
> This reminded me of a problem I once saw with the Java 7 (I think)
> compiler being unable to correctly infer the types when calling static
> generic methods.
> 
> Since I always used the Java 7 compiler to compile the code and as I
> saw the javac warning above, I got curious.
> 
> The thing is: the code compiles just fine with an actual Java 7
> compiler (OpenJDK 1.7.0_85), but not when using a Java 8 compiler
> (OpenJDK 1.8.0_60) with the `-source 1.7` switch.
> 

Ah, indeed. I'm using the Oracle Java 8 JDK and not OpenJDK.

>> But if the compile issue can be fixed we can probably require at least
>> Java 7. I think Java 6 is dangerous.
> 
> I'm still thinking about this. Maybe you're right. But still, the code
> is valid 1.7 source code. Should we prevent that code from compiling
> on Java 7 to protect users from using an obsolete JVM? I don't think
> this is the right place to control that.
> 

No, we can probably depend on Java 7, but we should update the README
that it doesn't compile properly on Oracle's JDK and you should use OpenJDK.

Wido

> --
> Claudio
> 

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

[libvirt] [PATCH] util: Add win32 version of virFileUnlink

2015-09-07 Thread Martin Kletzander 
Commit 35847860f65f Added the virFileUnlink function, but failed to add
a version for mingw build, causing the following error:

  Cannot export virFileUnlink: symbol not defined

Signed-off-by: Martin Kletzander 
---
 src/util/virfile.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 408d2d912f13..75819d9c8bd7 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2637,6 +2637,20 @@ virDirCreate(const char *path ATTRIBUTE_UNUSED,

 return -ENOSYS;
 }
+
+int
+virFileUnlink(const char *path,
+  uid_t uid ATTRIBUTE_UNUSED,
+  gid_t gid ATTRIBUTE_UNUSED)
+{
+if (unlink(path) < 0) {
+virReportSystemError(errno, _("Unable to unlink path '%s'"),
+ path);
+return -1;
+}
+
+return 0;
+}
 #endif /* WIN32 */

 /**
-- 
2.5.1

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

Re: [libvirt] [PATCH v2 1/2] virsh: Slightly rework cmdDomblklist

2015-09-07 Thread Michal Privoznik 
On 07.09.2015 09:23, Martin Kletzander wrote:
> On Wed, Sep 02, 2015 at 05:58:18PM +0200, Michal Privoznik wrote:
>> Let's move some variables from an inside loop to global function
>> declaration header block. It's going to be easier for next
>> patches. At the same time, order the cleanup calls at the
>> function's end so it's easier to track which variables are freed
>> and which not.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>> tools/virsh-domain-monitor.c | 29 ++---
>> 1 file changed, 14 insertions(+), 15 deletions(-)
>>
> 
> I did not follow the discussion around 2/2, but this one does not do
> any functional change and cleans up the code nicely, so ACK ... [1]

Pushed, thanks.

> 
>> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
>> index 340a8e2..d4e500b 100644
>> --- a/tools/virsh-domain-monitor.c
>> +++ b/tools/virsh-domain-monitor.c
>> @@ -549,24 +544,28 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
>> if (details) {
>> vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device,
>>  target, source ? source : "-");
>> -VIR_FREE(type);
>> -VIR_FREE(device);
>> } else {
>> vshPrint(ctl, "%-10s %s\n", target, source ? source : "-");
>> }
>>
>> -VIR_FREE(target);
>> VIR_FREE(source);
>> +VIR_FREE(target);
>> +VIR_FREE(device);
>> +VIR_FREE(type);
>> }
>>
>> ret = true;
>>
>>  cleanup:
>> +VIR_FREE(source);
>> +VIR_FREE(target);
>> +VIR_FREE(device);
>> +VIR_FREE(type);
>> VIR_FREE(disks);
>> -virDomainFree(dom);
>> -VIR_FREE(xml);
>> -xmlFreeDoc(xmldoc);
>> xmlXPathFreeContext(ctxt);
>> +xmlFreeDoc(xmldoc);
>> +VIR_FREE(xml);
>> +virDomainFree(dom);
> 
> [1] ... even though I don't understand why these are reordered.
> 

It's explained in the commit message. I always felt like cleanup should
follow the order laid out by variable definition at function beginning,
or in reversed order. If it is random, it's harder to spot a leaking
variable.

Michal

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

[libvirt] [PATCH] docs: Remove last use of double semicolon in Makefile

2015-09-07 Thread Martin Kletzander 
Double semicolons have special meaning in makefiles, but they would have
to be combined with other rules witch such separators in order to be
used as intended.  Since there are no other rules like that, let's
clean it up.

Signed-off-by: Martin Kletzander 
---
 docs/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index daf37b6b95de..bfae35e0ddba 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -204,7 +204,7 @@ todo:
rm -f todo.html.in
$(MAKE) todo.html

-hvsupport.html:: $(srcdir)/hvsupport.html.in
+hvsupport.html: $(srcdir)/hvsupport.html.in

 $(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.pl $(api_DATA) \
$(top_srcdir)/src/libvirt_public.syms \
-- 
2.5.1

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

Re: [libvirt] [PATCH] cpu: Introduce IvyBridge CPU model

2015-09-07 Thread Martin Kletzander 

On Fri, Sep 04, 2015 at 03:43:20PM +0200, Jiri Denemark wrote:

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

Signed-off-by: Jiri Denemark 
---
src/cpu/cpu_map.xml | 49 +
1 file changed, 49 insertions(+)



It would be easier to at least have the QEMU commit that adds the same
model in their codebase, but I see the vast information in the commit
message already took you some time to compose ;)  So I compared it to
2f9ac42acf4602453d5839221df6cc7cabc3355e and found out it adds the
same and nothing is missing, so ACK.


diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index d2469ee..51aa899 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -869,6 +869,55 @@
  


+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+

  
  
--
2.5.1

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


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

[libvirt] [PATCH] virsh: Teach attach-interface to --print-xml

2015-09-07 Thread Michal Privoznik 
We have the same argument to many other commands that produce an
XML based on what user typed. But unfortunately attach-interface
was missing it. Maybe nobody hasn't needed it yet. Well, I did
just now.

Signed-off-by: Michal Privoznik 
---
 tools/virsh-domain.c | 20 
 tools/virsh.pod  |  4 
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b029b65..516a51e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -862,6 +862,10 @@ static const vshCmdOptDef opts_attach_interface[] = {
  .type = VSH_OT_BOOL,
  .help = N_("affect current domain")
 },
+{.name = "print-xml",
+ .type = VSH_OT_BOOL,
+ .help = N_("print XML document rather than attach the interface")
+},
 {.name = NULL}
 };
 
@@ -938,9 +942,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 if (live)
 flags |= VIR_DOMAIN_AFFECT_LIVE;
 
-if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-return false;
-
 if (persistent &&
 virDomainIsActive(dom) == 1)
 flags |= VIR_DOMAIN_AFFECT_LIVE;
@@ -1051,6 +1052,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 virBufferAddLit(, "\n");
 }
 
+virBufferAdjustIndent(, -2);
 virBufferAddLit(, "\n");
 
 if (virBufferError()) {
@@ -1060,6 +1062,15 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 
 xml = virBufferContentAndReset();
 
+if (vshCommandOptBool(cmd, "print-xml")) {
+vshPrint(ctl, "%s", xml);
+functionReturn = true;
+goto cleanup;
+}
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
 if (flags || current)
 ret = virDomainAttachDeviceFlags(dom, xml, flags);
 else
@@ -1075,7 +1086,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 }
 
  cleanup:
-virDomainFree(dom);
+if (dom)
+virDomainFree(dom);
 virBufferFreeAndReset();
 return functionReturn;
 }
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 83c445d3..0212e7a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2507,6 +2507,7 @@ Likewise, I<--shareable> is an alias for I<--mode 
shareable>.
 [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
 [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
 [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>]
+[I<--print-xml>]
 
 Attach a new network interface to the domain.  I can be
 I to indicate connection via a libvirt virtual network, or
@@ -2536,6 +2537,9 @@ kilobytes in a single burst at I speed as described 
in the
 Network XML documentation at
 L.
 
+If I<--print-xml> is specified, then the XML of the interface that would be
+attached is printed instead.
+
 If I<--live> is specified, affect a running domain.
 If I<--config> is specified, affect the next startup of a persistent domain.
 If I<--current> is specified, affect the current domain state.
-- 
2.4.6

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

Re: [libvirt] [PATCH] cpu: Introduce IvyBridge CPU model

2015-09-07 Thread Jiri Denemark 
On Mon, Sep 07, 2015 at 11:16:11 +0200, Martin Kletzander wrote:
> On Fri, Sep 04, 2015 at 03:43:20PM +0200, Jiri Denemark wrote:
> >https://bugzilla.redhat.com/show_bug.cgi?id=1254420
> >
> >Signed-off-by: Jiri Denemark 
> >---
> > src/cpu/cpu_map.xml | 49 +
> > 1 file changed, 49 insertions(+)
> >
> 
> It would be easier to at least have the QEMU commit that adds the same
> model in their codebase, but I see the vast information in the commit
> message already took you some time to compose ;)  So I compared it to
> 2f9ac42acf4602453d5839221df6cc7cabc3355e and found out it adds the
> same and nothing is missing, so ACK.

Yeah, this commit message was especially hard to compose. Anyway, I
looked at the current definition of IvyBridge instead of searching for
the commit which introduced it so I really didn't have much more to add
:-)

Thanks and pushed.

Jirka

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

[libvirt] [PATCHv2] qemu: Validate address type when attaching a disk device.

2015-09-07 Thread Ruifeng Bian 
https://bugzilla.redhat.com/show_bug.cgi?id=1257844

Attach-device can hotplug a virtio disk device with any address type now,
it need to validate the address type before the attachment.

Coldplug a scsi disk device without checking the address type in current
version, this patch also fix the scsi disk problem.
---
 src/qemu/qemu_driver.c  |  8 
 src/qemu/qemu_hotplug.c | 18 ++
 2 files changed, 26 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91eb661..af926fc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8050,6 +8050,14 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
 if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
 if (virDomainDefAddImplicitControllers(vmdef) < 0)
 return -1;
+/* scsi disk should have an address type of driver */
+if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
+(disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("scsi disk cannot have an address of type 
'%s'"),
+   
virDomainDeviceAddressTypeToString(disk->info.type));
+return -1;
+}
 if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
 return -1;
 break;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 63fafa6..4226650 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -335,6 +335,24 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, 
priv->qemuCaps,
 disk->dst))
 goto cleanup;
+
+/* virtio device should either have a ccw or pci address */
+if (qemuDomainMachineIsS390CCW(vm->def) &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
+if (!virDomainDeviceAddressIsValid(>info,
+   
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("device cannot be attached without a valid 
CCW address"));
+goto cleanup;
+}
+} else {
+if (!virDomainDeviceAddressIsValid(>info,
+   
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("device cannot be attached without a valid 
PCI address"));
+goto cleaup;
+}
+}
 }
 
 for (i = 0; i < vm->def->ndisks; i++) {
-- 
2.4.3

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

Re: [libvirt] [PATCHv3 0/2] Added waiting for DAD to finish for bridge address.

2015-09-07 Thread Maxim Perevedentsev 

Hello everyone!

I'd like to remind I'm still waiting for comments on these patches.

On 08/26/2015 01:28 PM, Maxim Perevedentsev wrote:

Thank you! Waiting in hope you have not forgotten. :)

On 08/17/2015 10:11 PM, Laine Stump wrote:

On 08/17/2015 10:48 AM, Maxim Perevedentsev wrote:

Hello guys!

Just a humble reminder of pending request :-)
Any suggestions about patches maybe?


Sorry for the delay. Pretty much everybody (including me) is at KVM
Forum this week. I'll try to make some comments on the patches as soon
as I can.


--
Your sincerely,
Maxim Perevedentsev

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

Re: [libvirt] [PATCH sandbox 2/4] Sync and unmount filesystems during shutdown

2015-09-07 Thread Cedric Bosdonnat 
On Fri, 2015-09-04 at 12:40 +0100, Daniel P. Berrange wrote:
> To ensure that all pending I/O for filesytems backed by block
> devices is flushed to disk, it is important to sync and unmount
> the filesystems during shutdown. To avoid relying on the kernel
> reboot-on-panic behaviour, we also explicitly call reboot to
> power off the guest.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-sandbox/libvirt-sandbox-init-common.c | 166 
> ++
>  libvirt-sandbox/libvirt-sandbox-init-qemu.c   |   1 +
>  2 files changed, 167 insertions(+)
> 
> diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
> b/libvirt-sandbox/libvirt-sandbox-init-common.c
> index 760509f..a3b4687 100644
> --- a/libvirt-sandbox/libvirt-sandbox-init-common.c
> +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
> @@ -44,6 +44,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include "libvirt-sandbox-rpcpacket.h"
>  
> @@ -52,6 +54,8 @@ static gboolean verbose = FALSE;
>  static int sigwrite;
>  
>  #define ATTR_UNUSED __attribute__((__unused__))
> +static void sync_data(void);
> +static void umount_fs(void);
>  
>  static void sig_child(int sig ATTR_UNUSED)
>  {
> @@ -598,6 +602,14 @@ static GVirSandboxRPCPacket 
> *gvir_sandbox_encode_exit(int status,
>  pkt->header.type = GVIR_SANDBOX_PROTOCOL_TYPE_MESSAGE;
>  pkt->header.serial = serial;
>  
> +/* The host may destroy the guest any time after receiving
> + * the exit code messages. So although the main() has code
> + * to sync + unmount we can't rely on that running. So we
> + * opportunistically sync + unmount here too.
> + */
> +sync_data();
> +umount_fs();
> +
>  if (!gvir_sandbox_rpcpacket_encode_header(pkt, error))
>  goto error;
>  if (!gvir_sandbox_rpcpacket_encode_payload_msg(pkt,
> @@ -613,7 +625,151 @@ static GVirSandboxRPCPacket 
> *gvir_sandbox_encode_exit(int status,
>  return NULL;
>  }
>  
> +/* Copied & adapted from libguestfs daemon/sync.c under LGPLv2+ */
> +static void sync_data(void)
> +{
> +  DIR *dir;
> +  struct dirent *d;
> +  char *dev_path;
> +  int fd;
> +
> +  if (debug)
> +  fprintf(stderr, "Syncing data\n");
> +  sync();
> +
> +  /* On Linux, sync(2) doesn't perform a barrier, so qemu (which may
> +   * have a writeback cache, even with cache=none) will still have
> +   * some unwritten data.  Force the data out of any qemu caches, by
> +   * calling fsync on all block devices.  Note we still need the
> +   * call to sync above in order to schedule the writes.
> +   * Thanks to: Avi Kivity, Kevin Wolf.
> +   */
> +
> +  if (!(dir = opendir("/dev"))) {
> +  fprintf(stderr, "opendir: /dev failed %s\n", strerror(errno));
> +  return;
> +  }
> +
> +  for (;;) {
> +  errno = 0;
> +  d = readdir(dir);
> +  if (!d)
> +  break;
> +
> +  if (!(g_str_has_prefix(d->d_name, "sd") ||
> +g_str_has_prefix(d->d_name, "hd") ||
> +g_str_has_prefix(d->d_name, "ubd") ||
> +g_str_has_prefix(d->d_name, "vd") ||
> +g_str_has_prefix(d->d_name, "sr"))) {
> +  continue;
> +  }
> +
> +  dev_path = g_strdup_printf("/dev/%s", d->d_name);
> +
> +  if (debug)
> +  fprintf(stderr, "Syncing fd %s\n", dev_path);
> +  if ((fd = open(dev_path, O_RDONLY)) < 0) {
> +  fprintf(stderr, "cannot open %s: %s\n", dev_path,
> +  strerror(errno));
> +  g_free(dev_path);
> +  continue;
> +  }
> +
> +  /* fsync the device. */
> +  if (debug) {
> +  fprintf(stderr, "fsync %s\n", dev_path);
> +  }
> +
> +  if (fsync(fd) < 0) {
> +  fprintf(stderr, "failed to fsync %s: %s\n",
> +  dev_path, strerror(errno));
> +  }
> +  if (close(fd) < 0) {
> +  fprintf(stderr, "failed to close %s: %s\n",
> +  dev_path, strerror(errno));
> +  }
> +  g_free(dev_path);
> +  }
> +
> +  /* Check readdir didn't fail */
> +  if (errno != 0) {
> +  fprintf(stderr, "Failed to read /dev: %s\n",
> +  strerror(errno));
> +  }
> +
> +  /* Close the directory handle */
> +  if (closedir(dir) < 0) {
> +  fprintf(stderr, "Failed to block /dev: %s\n",
> +  strerror(errno));
> +  }
> +
> +  if (debug)
> +  fprintf(stderr, "Syncing complete\n");
> +}
> +
> +
> +static int
> +compare_longest_first (gconstpointer vp1, gconstpointer vp2)
> +{
> +int n1 = strlen(vp1);
> +int n2 = strlen(vp2);
> +return n2 - n1;
> +}
> +
> +
> +/* Copied & adapted from libguestfs daemon/sync.c under LGPLv2+ */
> +static void umount_fs(void)
> +{
> +FILE *fp;
> +struct mntent *m;
> +GList *mounts = NULL, *tmp;
> +
> +if (debug)
> +fprintf(stderr, "Unmounting all filesystems\n");
> +if (!(fp = setmntent ("/proc/mounts", "r"))) {
> +fprintf(stderr, "Failed to open /proc/mounts: %s\n",
> +

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-09-07 Thread Daniel P. Berrange 
On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote:
> We already have a fuse mount to reflect the cgroup memory restrictions
> in the container. This commit adds the same for the number of available
> CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the
> container's cpuinfo.

So this (re-)raises some interesting / difficult questions that I'm
not sure we have a good answer to.

The main concern is that actually this is not really a problem specific
to containers, rather it is related to cgroup resource confinement.
ie the cgroup has confined a process(es) to a set of CPUs are the process
is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being
increasingly widely used in Linux, particularly since systemd, so pretty
much any process has to expect that it can be confined to a subset of
CPUs.

IOW, any application using /proc/cpuinfo to determine "available"
resource is already broken, even when run on bare metal. The same also
applies to the use of /proc/meminfo, which we previously faked via
fuse.

So the question is whether we should invest time trying to fake the
/proc/cpuinfo in containers, when any apps we'd be fixing are already
broken in bare metal. Apps might have avoided /proc/cpuinfo and instead
be trying /sys/devices/system/cpu/ which your patch isn't trying to
fake. This is just as broken, because sysfs doesn't reflect cgroup
confinement either.

I think what is ultimately needed for applications is some kind of
libresource.so library that they can use to query what resources
are available in their compute environment, which can intelligently
query cgroups directly, and ignore the legacy /proc & /sys interfaces
for counting memory / cpu availability. I don't think that's something
that libvirt should solve - if anything it could be systemd, or a
standalone project.

So I'm increasingly convinced that LXC should not try to fake out
any /proc & /sys file content, and instead document the limitations.
I'm also thinking that we should kill off our existing meminfo fake
fuse at some point.

The more minor concern I have is around the implementation. AFAIR, the
/proc/cpuinfo file contents is not standardized across architectures,
so I'm concerned whether your parsing code is robust on non-x86 arches.

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

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

Re: [libvirt] [sandbox PATCH v4 01/21] Add virt-sandbox-image

2015-09-07 Thread Daniel P. Berrange 
On Fri, Aug 28, 2015 at 01:47:29PM +, Eren Yagdiran wrote:
> From: Daniel P Berrange 
> 
> virt-sandbox-image.py is a python script that lets you download Docker
> images easily. It is a proof of concept code and consumes Docker Rest API.
> ---
>  po/POTFILES.in   |   1 +
>  virt-sandbox-image/virt-sandbox-image.py | 394 
> +++
>  2 files changed, 395 insertions(+)
>  create mode 100644 virt-sandbox-image/virt-sandbox-image.py

I'm thinking that this file would be better off going in

   libvirt-sandbox/image/cli.py

and then adding a bin/virt-sandbox-image file that merely does

  #!/usr/bin/python
  # -*- coding: utf-8 -*-

  from libvirt_sandbox.image import cli
  import sys

  if __name__ == '__main__':
 sys.exit(cli.main())

and also updating libvirt-sandbox.spec.in to hold the new file


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

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

Re: [libvirt] [PATCH v2 2/2] virsh: Enhance the detailed output of domblklist for networked source

2015-09-07 Thread Lin Ma 


在 2015年09月04日 18:53, John Ferlan 写道:


On 09/04/2015 04:26 AM, Michal Privoznik wrote:

On 03.09.2015 22:47, John Ferlan wrote:


On 09/02/2015 11:58 AM, Michal Privoznik wrote:

From: Lin Ma 

Format & output more detailed information about networked source

e.g: The output without the patch:
$ virsh domblklist $DOMAIN --details
Type   Device Target Source

networkdisk   vdatest-pool/image
networkdisk   vdbiqn.2015-08.org.example:sn01/0
networkdisk   vdc/image.raw
networkdisk   vdd-
networkdisk   vde-
networkdisk   vdfimage1
networkdisk   vdgtest-volume/image.raw

The output with the patch:
$ virsh domblklist $DOMAIN --details
Type   Device Target Source

networkdisk   vdarbd://monitor1.example.org:6321/test-pool/image
networkdisk   vdb
iscsi://192.168.124.200:3260/iqn.2015-08.org.example:sn01/0
networkdisk   vdchttp://192.168.124.200:80/image.raw
networkdisk   vddnbd+unix:///var/run/nbdsock
networkdisk   vdenbd://192.168.124.200:12345
networkdisk   vdfsheepdog://192.168.124.200:6000/image1
networkdisk   vdggluster://192.168.124.200/test-volume/image.raw

Is the goal to just format in some "standard" format or to use the
format that would be used by qemu in the command line?

While it's the former, I think we should at least cover asses and
document that these strings have no special meaning and can change later
if we find a better way of expressing them. Or should we?


Since the Source path is provided whether or not --details is supplied,
I guess I'd be concerned if we've ever made any "guarantees" about the
format of the output for default displays and what a change like this
could break. I can tell you that when there was a change to add an extra
leading space to some display output, there were virt-tests that just
began failing because the difference between seeing "# ..." and " # ..."
in the output wasn't handled properly.

Anyway, the man page says:

Print a table showing the brief information of all block devices
associated with I. If I<--inactive> is specified, query the
block devices that will be used on the next boot, rather than those
currently in use by a running domain. If I<--details> is specified,
disk type and device value will also be printed. Other contexts
that require a block device name (such as I or
I for disk snapshots) will accept either target
or unique source names printed by this command.


Which a naive user could be led to believe that by grabbing the value in
the "Source" column (such as "nbd://192.168.124.200:12345") they could
feed that into "virsh domblkinfo $dom $Source" and it would work. In
fact, someone that can write scripts better than I could be currently
stripping that last field off and using it as input to their domblkinfo
command in order to get the Capacity, Allocation, Physical values in
some form. Yes, of course those would be "broken" today for network.
Since the test environment is already set up, Lin Ma can you at least
see what happens for the various formats if one just cut-n-pasted that
column for domblkinfo?


For networked disks which were attached directly, domblkinfo can't 
recognize them:

virsh domblkinfo $dom $Target
will output the error message with related protocol:
"error: internal error: missing storage backend for network files using 
%s protocol"


virsh domblkinfo $dom $Source
will output "error: invalid argument: invalid path $Source not assigned 
to domain"



One option/adjustment perhaps is we only print the "details" of the
network source if --details is provided (and document it).  Or we could
add something like the following after the first sentence to virsh.pod
(for the CYA needs):

For networked storage the Source displayed uses the domain XML to
extract source protocol, transport, host name, host port, and source
name or socket data in order to format and display in a standard manner
starting with the protocol, such as:

"$protocol[+$transport]://[$host:$port][{/$name|:$socket}]"


That could be ugly difficult to display and I don't see any other
current example in a quick scan through virsh.pod.

John



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

[libvirt] VMware: Map vpx:// to dcPath

2015-09-07 Thread Richard W.M. Jones 

vpx:// paths looks like this:

  vpx://vcenter.example.com/MyFolder/MyDatacenter/MyCluster/esxi

but to connect to the datastore to read the underlying disk image,
libguestfs must form a URL like this:

  
https://vcenter.example.com/folder/data/guest/guest-flat.vmdk?dcPath=MyFolder/MyDatacenter=datastore

All parts of this URL can be worked out from the URL or the libvirt
XML *except* the ?dcPath=... parameter.

The problem is that dcPath isn't a straight mapping from the vpx://
path.  The particular problem is that if there is a cluster name in
the path (eg 'MyCluster') it appears that we have to remove it.  ie:

  dcPath=MyFolder/MyDatacenter/MyCluster   - does not work
  dcPath=MyFolder/MyDatacenter - works

That would be OK if there was always a cluster name at the end of the
path, but there isn't.  Clusters are completely optional, and AFAIK
you can't tell if something is a cluster path element just by
examining the name, since clusters can be given arbitrary names by the
vCenter admin.

So:

(1) Is there something I'm missing here?  Maybe the libvirt VMware
driver presents this information already and I'm just missing it?

(2) Can we add some way to more easily map from vpx:// paths to
datastore URLs?  The whole process is very complex even without the
ambiguity - it takes a couple of pages of code to do the mapping.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

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

Re: [libvirt] [sandbox PATCH v4 03/21] Image: Add Hooking Mechanism

2015-09-07 Thread Daniel P. Berrange 
On Fri, Aug 28, 2015 at 01:47:31PM +, Eren Yagdiran wrote:
> Any custom source provider can be added to virt-sandbox-image as a source
> ---
>  .gitignore   |  1 +
>  bin/Makefile.am  | 16 
>  bin/virt-sandbox-image.in|  3 +++
>  configure.ac |  2 ++
>  virt-sandbox-image/Makefile.am   | 13 +
>  virt-sandbox-image/sources/Source.py | 27 +++
>  virt-sandbox-image/sources/__init__.py   | 26 ++
>  virt-sandbox-image/virt-sandbox-image.py | 15 +--
>  8 files changed, 93 insertions(+), 10 deletions(-)
>  create mode 100644 bin/virt-sandbox-image.in
>  create mode 100644 virt-sandbox-image/Makefile.am
>  create mode 100644 virt-sandbox-image/sources/Source.py
>  create mode 100644 virt-sandbox-image/sources/__init__.py
>  mode change 100644 => 100755 virt-sandbox-image/virt-sandbox-image.py
> 
> diff --git a/.gitignore b/.gitignore
> index f77ea12..ef5b5aa 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -56,3 +56,4 @@ bin/virt-sandbox
>  bin/virt-sandbox-service-util
>  build/
>  bin/*.1
> +bin/virt-sandbox-image
> diff --git a/bin/Makefile.am b/bin/Makefile.am
> index 416f86f..df4c7dc 100644
> --- a/bin/Makefile.am
> +++ b/bin/Makefile.am
> @@ -3,7 +3,11 @@ bin_PROGRAMS = virt-sandbox
>  
>  libexec_PROGRAMS = virt-sandbox-service-util
>  
> -bin_SCRIPTS = virt-sandbox-service
> +bin_SCRIPTS = virt-sandbox-service \
> +  virt-sandbox-image
> +
> +virt-sandbox-image: virt-sandbox-image.in
> + sed -e 's,[@]pkgpythondir[@],$(pkgpythondir),g' < $< >$@
>  
>  virtsandboxcompdir = $(datarootdir)/bash-completion/completions/
>  
> @@ -20,8 +24,11 @@ POD_FILES = \
>   virt-sandbox-service-reload.pod \
>   virt-sandbox-service-upgrade.pod \
>   $(NULL)
> -EXTRA_DIST = $(bin_SCRIPTS) $(POD_FILES) 
> virt-sandbox-service-bash-completion.sh virt-sandbox-service.logrotate
> -EXTRA_DIST += virt-sandbox-service-bash-completion.sh
> +EXTRA_DIST = virt-sandbox-service \
> + virt-sandbox-image.in \
> + $(POD_FILES) \
> + virt-sandbox-service-bash-completion.sh \
> + virt-sandbox-service.logrotate
>  
>  man1_MANS = \
>   virt-sandbox.1 \
> @@ -64,7 +71,8 @@ virt-sandbox-service-reload.1: 
> virt-sandbox-service-reload.pod Makefile
>  virt-sandbox-service-upgrade.1: virt-sandbox-service-upgrade.pod Makefile
>   $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
>  
> -CLEANFILES = $(man1_MANS)
> +CLEANFILES = $(man1_MANS) \
> + virt-sandbox-image
>  
>  virt_sandbox_SOURCES = virt-sandbox.c
>  virt_sandbox_CFLAGS = \
> diff --git a/bin/virt-sandbox-image.in b/bin/virt-sandbox-image.in
> new file mode 100644
> index 000..732bb38
> --- /dev/null
> +++ b/bin/virt-sandbox-image.in
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +
> +exec "@pkgpythondir@/virt-sandbox-image.py" "$@"

Per my comment on patch 1, we can actually do this in python

#!/usr/bin/python
# -*- coding: utf-8 -*-

from libvirt_sandbox.image import cli
import sys

if __name__ == '__main__':
   sys.exit(cli.main())


which nicely avoids the need to have a .in file text
substitution.


> diff --git a/virt-sandbox-image/sources/Source.py 
> b/virt-sandbox-image/sources/Source.py
> new file mode 100644
> index 000..508ca80
> --- /dev/null
> +++ b/virt-sandbox-image/sources/Source.py

I'm thinking these will be best in libvirt-sandbox/image/sources/Source.py
so we get a python import module of 'libvirt_sandbox.image.sources.Source"

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

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

Re: [libvirt] [PATCH] target-i386: Enable "check" mode by default

2015-09-07 Thread Paolo Bonzini 


On 26/08/2015 18:50, Eduardo Habkost wrote:
> Current default behavior of QEMU is to silently disable features that
> are not supported by the host when a CPU model is requested in the
> command-line. This means that in addition to risking breaking guest ABI
> by default, we are silent about it.
> 
> I would like to enable "enforce" by default, but this can easily break
> existing production systems because of the way libvirt makes assumptions
> about CPU models today (this will change in the future, once QEMU
> provide a proper interface for checking if a CPU model is runnable).
> 
> But there's no reason we should be silent about it. So, change
> target-i386 to enable "check" mode by default so at least we have some
> warning printed to stderr (and hopefully logged somewhere) when QEMU
> disables a feature that is not supported by the host system.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  target-i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This adds a warning to TCG with the default qemu32/qemu64 CPU models,
due to lack of DE implementation in TCG.  It can be fixed before
release, so this patch is okay.  But please remind me to do it, or
implement it yourself. :)

Paolo

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

Re: [libvirt] [PATCH sandbox 4/4] Don't close immediately when getting EOF on RPC console

2015-09-07 Thread Cedric Bosdonnat 
On Fri, 2015-09-04 at 12:40 +0100, Daniel P. Berrange wrote:
> The RPC console is closed when the libvirt-sandbox-init-common
> binary reports the exit of the guest process. We still have
> some cleanup code that runs in the guest, for example, syncing
> and ummounting filesystems. We want to be able to see debug
> and/or error messages from this code, so we should not quit
> until we get a close on that console. This should happen a
> few ms after the close on the RPC console, but just in case
> something causes shutdown to hang, we have a delayed timer
> registered.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  bin/virt-sandbox.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
> index 195515f..332e53e 100644
> --- a/bin/virt-sandbox.c
> +++ b/bin/virt-sandbox.c
> @@ -34,6 +34,22 @@ static gboolean do_close(GVirSandboxConsole *con 
> G_GNUC_UNUSED,
>  return FALSE;
>  }
>  
> +static gboolean do_delayed_close(gpointer opaque)
> +{
> +GMainLoop *loop = opaque;
> +g_main_loop_quit(loop);
> +return FALSE;
> +}
> +
> +static gboolean do_pending_close(GVirSandboxConsole *con G_GNUC_UNUSED,
> + gboolean error G_GNUC_UNUSED,
> + gpointer opaque)
> +{
> +GMainLoop *loop = opaque;
> +g_timeout_add(2000, do_delayed_close, loop);
> +return FALSE;
> +}
> +
>  static gboolean do_exited(GVirSandboxConsole *con G_GNUC_UNUSED,
>int status,
>gpointer opaque)
> @@ -256,7 +272,12 @@ int main(int argc, char **argv) {
> error && error->message ? error->message : _("Unknown 
> failure"));
>  goto cleanup;
>  }
> -g_signal_connect(con, "closed", (GCallback)do_close, loop);
> +/* We don't close right away - we want to ensure we read any
> + * final debug info from the log console. We should get an
> + * EOF on that console which will trigger the real close,
> + * but we schedule a timer just in case.
> + */
> +g_signal_connect(con, "closed", (GCallback)do_pending_close, loop);
>  g_signal_connect(con, "exited", (GCallback)do_exited, );
>  
>  if (!(gvir_sandbox_console_attach_stdio(con, ))) {

ACK

--
Cedric


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

Re: [libvirt] [PATCH sandbox 3/4] Fix passing of strace option to guest kernel

2015-09-07 Thread Cedric Bosdonnat 
On Fri, 2015-09-04 at 12:40 +0100, Daniel P. Berrange wrote:
> The libvirt-sandbox-init-qemu command expects to see 'strace='
> or 'strace=some,list,of,syscalls' but we only passed 'strace'.
> This meant strace could never be enabled.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-sandbox/libvirt-sandbox-builder-machine.c | 7 ++-
>  libvirt-sandbox/libvirt-sandbox-init-qemu.c   | 2 +-
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c 
> b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> index a458882..a142f68 100644
> --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> @@ -239,11 +239,8 @@ static gchar 
> *gvir_sandbox_builder_machine_cmdline(GVirSandboxConfig *config G_G
>  g_string_append(str, " quiet loglevel=0");
>  
>  if ((tmp = getenv("LIBVIRT_SANDBOX_STRACE"))) {
> -g_string_append(str, " strace");
> -if (!g_str_equal(tmp, "1")) {
> -g_string_append(str, "=");
> -g_string_append(str, tmp);
> -}
> +g_string_append(str, " strace=");
> +g_string_append(str, tmp);
>  }
>  
>  /* These make boot a little bit faster */
> diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c 
> b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
> index 8bde224..bbe70ad 100644
> --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
> +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
> @@ -414,7 +414,7 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED)
>  args[narg++] = "/tmp/sandbox.log";
>  args[narg++] = "-f";
>  args[narg++] = "-ff";
> -if (strace) {
> +if (strace && STRNEQ(strace, "1")) {
>  args[narg++] = "-e";
>  args[narg++] = strace;
>  }

ACK

--
Cedric

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

Re: [libvirt] [PATCH] docs: Drop unused rule for internals/%.html.tmp target

2015-09-07 Thread Martin Kletzander 

On Wed, Sep 02, 2015 at 09:27:00AM +0200, Guido Günther wrote:

On Tue, Sep 01, 2015 at 03:27:05PM +0200, Michal Privoznik wrote:
[..snip..]

Guido, I think the original problem you're seeing is just an ordering
problem. If you revert you patch, and apply this one:


This gives the same error.  Feel free to revert my change and I keep
the patch in Debian until we tracked down the root cause and don't
interfere with the 1.2.19 release.



The release was done without the revert, but newer Debian is more
important then super-old CentOS.  The problem is that we still have
broken subsites (the internal pages).  I tried reproducing your
problem on our debian installation and I couldn't.  I have no idea how
the versions go, etc.  But I really badly want to fix this.  What are
the versions and steps to reproduce for the original problem you were
trying to fix?  I'm starting fresh installation to finally fix this
once and for all.

Martin


Cheers,
-- Guido

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


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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-09-07 Thread Serge Hallyn 
Quoting Daniel P. Berrange (berra...@redhat.com):
> On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote:
> > We already have a fuse mount to reflect the cgroup memory restrictions
> > in the container. This commit adds the same for the number of available
> > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the
> > container's cpuinfo.
> 
> So this (re-)raises some interesting / difficult questions that I'm
> not sure we have a good answer to.
> 
> The main concern is that actually this is not really a problem specific
> to containers, rather it is related to cgroup resource confinement.
> ie the cgroup has confined a process(es) to a set of CPUs are the process
> is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being
> increasingly widely used in Linux, particularly since systemd, so pretty
> much any process has to expect that it can be confined to a subset of
> CPUs.
> 
> IOW, any application using /proc/cpuinfo to determine "available"
> resource is already broken, even when run on bare metal. The same also
> applies to the use of /proc/meminfo, which we previously faked via
> fuse.
> 
> So the question is whether we should invest time trying to fake the
> /proc/cpuinfo in containers, when any apps we'd be fixing are already
> broken in bare metal. Apps might have avoided /proc/cpuinfo and instead
> be trying /sys/devices/system/cpu/ which your patch isn't trying to
> fake. This is just as broken, because sysfs doesn't reflect cgroup
> confinement either.
> 
> I think what is ultimately needed for applications is some kind of
> libresource.so library that they can use to query what resources

Does anyone remember who it was that announced an effort to this
end a year or two ago, or know what the status of it is?

> are available in their compute environment, which can intelligently
> query cgroups directly, and ignore the legacy /proc & /sys interfaces
> for counting memory / cpu availability. I don't think that's something
> that libvirt should solve - if anything it could be systemd, or a
> standalone project.
> 
> So I'm increasingly convinced that LXC should not try to fake out
> any /proc & /sys file content, and instead document the limitations.
> I'm also thinking that we should kill off our existing meminfo fake
> fuse at some point.
> 
> The more minor concern I have is around the implementation. AFAIR, the
> /proc/cpuinfo file contents is not standardized across architectures,
> so I'm concerned whether your parsing code is robust on non-x86 arches.
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o- http://virt-manager.org :|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-09-07 Thread Serge Hallyn 
Quoting Daniel P. Berrange (berra...@redhat.com):
> On Mon, Sep 07, 2015 at 03:39:13PM +, Serge Hallyn wrote:
> > Quoting Daniel P. Berrange (berra...@redhat.com):
> > > On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote:
> > > > We already have a fuse mount to reflect the cgroup memory restrictions
> > > > in the container. This commit adds the same for the number of available
> > > > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the
> > > > container's cpuinfo.
> > > 
> > > So this (re-)raises some interesting / difficult questions that I'm
> > > not sure we have a good answer to.
> > > 
> > > The main concern is that actually this is not really a problem specific
> > > to containers, rather it is related to cgroup resource confinement.
> > > ie the cgroup has confined a process(es) to a set of CPUs are the process
> > > is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being
> > > increasingly widely used in Linux, particularly since systemd, so pretty
> > > much any process has to expect that it can be confined to a subset of
> > > CPUs.
> > > 
> > > IOW, any application using /proc/cpuinfo to determine "available"
> > > resource is already broken, even when run on bare metal. The same also
> > > applies to the use of /proc/meminfo, which we previously faked via
> > > fuse.
> > > 
> > > So the question is whether we should invest time trying to fake the
> > > /proc/cpuinfo in containers, when any apps we'd be fixing are already
> > > broken in bare metal. Apps might have avoided /proc/cpuinfo and instead
> > > be trying /sys/devices/system/cpu/ which your patch isn't trying to
> > > fake. This is just as broken, because sysfs doesn't reflect cgroup
> > > confinement either.
> > > 
> > > I think what is ultimately needed for applications is some kind of
> > > libresource.so library that they can use to query what resources
> > 
> > Does anyone remember who it was that announced an effort to this
> > end a year or two ago, or know what the status of it is?
> 
> I don't recall seeing any formal announcement about this, but I have
> had this exact same discussion with Red Hat folks involved with
> Docker and similar higher level container mgmt tools, so perhaps
> someone involved in those efforts is working on it ?

Ah, my memory was failing me, so took a bit of searching, but 

http://fabiokung.com/2014/03/13/memory-inside-linux-containers/

I can't find anything called 'libmymem', and in 2014 he said

https://github.com/docker/docker/issues/8427#issuecomment-58255159

so maybe this never went anywhere.

For the same reasons you cited above, and because everyeone is rolling
their own at fuse level, I still think that a libresource and patches
to proc tools to use them, is the right way to go.  We have no shortage
of sample code for the functions doing the actual work, between libvirt,
lxc, docker, etc :)

Should we just go ahead and start a libresource github project?

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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-09-07 Thread Andrea Bolognani 
On Mon, 2015-09-07 at 15:21 +0200, Cedric Bosdonnat wrote:
> > The more minor concern I have is around the implementation. AFAIR,
> > the
> > /proc/cpuinfo file contents is not standardized across
> > architectures,
> > so I'm concerned whether your parsing code is robust on non-x86
> > arches.
> 
> Hum... I didn't even know that file would change with arch'es.

Take a look at linuxNodeInfoCPUPopulate() in src/nodeinfo.c
for inspiration. Sharing the parsing code would also be nice.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-09-07 Thread Daniel P. Berrange 
On Mon, Sep 07, 2015 at 03:39:13PM +, Serge Hallyn wrote:
> Quoting Daniel P. Berrange (berra...@redhat.com):
> > On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote:
> > > We already have a fuse mount to reflect the cgroup memory restrictions
> > > in the container. This commit adds the same for the number of available
> > > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the
> > > container's cpuinfo.
> > 
> > So this (re-)raises some interesting / difficult questions that I'm
> > not sure we have a good answer to.
> > 
> > The main concern is that actually this is not really a problem specific
> > to containers, rather it is related to cgroup resource confinement.
> > ie the cgroup has confined a process(es) to a set of CPUs are the process
> > is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being
> > increasingly widely used in Linux, particularly since systemd, so pretty
> > much any process has to expect that it can be confined to a subset of
> > CPUs.
> > 
> > IOW, any application using /proc/cpuinfo to determine "available"
> > resource is already broken, even when run on bare metal. The same also
> > applies to the use of /proc/meminfo, which we previously faked via
> > fuse.
> > 
> > So the question is whether we should invest time trying to fake the
> > /proc/cpuinfo in containers, when any apps we'd be fixing are already
> > broken in bare metal. Apps might have avoided /proc/cpuinfo and instead
> > be trying /sys/devices/system/cpu/ which your patch isn't trying to
> > fake. This is just as broken, because sysfs doesn't reflect cgroup
> > confinement either.
> > 
> > I think what is ultimately needed for applications is some kind of
> > libresource.so library that they can use to query what resources
> 
> Does anyone remember who it was that announced an effort to this
> end a year or two ago, or know what the status of it is?

I don't recall seeing any formal announcement about this, but I have
had this exact same discussion with Red Hat folks involved with
Docker and similar higher level container mgmt tools, so perhaps
someone involved in those efforts is working on it ?


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

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

[libvirt] [PATCH] admin: Resolve leaked reference to private data

2015-09-07 Thread Erik Skultety 
Running valgrind on a very simplistic program consisting only from
opening and closing admin connection (virAdmConnect{Open,Close}) shows a
leak in remoteAdminPrivNew, because the last reference to privateData is
not decremented, thus the object won't be disposed. This patch unrefs
the privateData object once we closed the active connection to daemon,
making further use of this connection  useless.

==24577==at 0x4A089C7: calloc (in /usr/lib64/valgrind/vgpreload_***linux.so)
==24577==by 0x4E8835F: virAllocVar (viralloc.c:560)
==24577==by 0x4EDFA5C: virObjectNew (virobject.c:193)
==24577==by 0x4EDFBD4: virObjectLockableNew (virobject.c:219)
==24577==by 0x4C14DAF: remoteAdminPrivNew (libvirt-admin.c:152)
==24577==by 0x4C1537E: virAdmConnectOpen (libvirt-admin.c:308)
==24577==by 0x400BAD: main (listservers.c:39)

==24577== LEAK SUMMARY:
==24577==definitely lost: 80 bytes in 1 blocks
==24577==indirectly lost: 840 bytes in 6 blocks
==24577==  possibly lost: 0 bytes in 0 blocks
==24577==still reachable: 12,179 bytes in 199 blocks
==24577== suppressed: 0 bytes in 0 blocks
---
 src/libvirt-admin.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index b3fd0b3..5a4fc48 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -132,6 +132,7 @@ remoteAdminPrivFree(void *opaque)
 virAdmConnectPtr conn = opaque;
 
 remoteAdminConnectClose(conn);
+virObjectUnref(conn->privateData);
 }
 
 static remoteAdminPrivPtr
-- 
2.4.3

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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-09-07 Thread Cedric Bosdonnat 
On Mon, 2015-09-07 at 13:23 +0100, Daniel P. Berrange wrote:
> On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote:
> > We already have a fuse mount to reflect the cgroup memory restrictions
> > in the container. This commit adds the same for the number of available
> > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the
> > container's cpuinfo.
> 
> So this (re-)raises some interesting / difficult questions that I'm
> not sure we have a good answer to.
> 
> The main concern is that actually this is not really a problem specific
> to containers, rather it is related to cgroup resource confinement.
> ie the cgroup has confined a process(es) to a set of CPUs are the process
> is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being
> increasingly widely used in Linux, particularly since systemd, so pretty
> much any process has to expect that it can be confined to a subset of
> CPUs.

I agree.

> IOW, any application using /proc/cpuinfo to determine "available"
> resource is already broken, even when run on bare metal. The same also
> applies to the use of /proc/meminfo, which we previously faked via
> fuse.
> 
> So the question is whether we should invest time trying to fake the
> /proc/cpuinfo in containers, when any apps we'd be fixing are already
> broken in bare metal. Apps might have avoided /proc/cpuinfo and instead
> be trying /sys/devices/system/cpu/ which your patch isn't trying to
> fake. This is just as broken, because sysfs doesn't reflect cgroup
> confinement either.

I agree /sys/devices/system/cpu should be patched too... but it contains
much more subtle things to handle. At least I don't have a good enough
knowledge of that FS to fake it properly.

> I think what is ultimately needed for applications is some kind of
> libresource.so library that they can use to query what resources
> are available in their compute environment, which can intelligently
> query cgroups directly, and ignore the legacy /proc & /sys interfaces
> for counting memory / cpu availability. I don't think that's something
> that libvirt should solve - if anything it could be systemd, or a
> standalone project.

Ok, then not something that would be available in a reasonable time
frame unless we start it. Do you know if someone in another project is
caring about that problem?

> So I'm increasingly convinced that LXC should not try to fake out
> any /proc & /sys file content, and instead document the limitations.
> I'm also thinking that we should kill off our existing meminfo fake
> fuse at some point.

OK.

> The more minor concern I have is around the implementation. AFAIR, the
> /proc/cpuinfo file contents is not standardized across architectures,
> so I'm concerned whether your parsing code is robust on non-x86 arches.

Hum... I didn't even know that file would change with arch'es.

--
Cedric

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

Re: [libvirt] [sandbox PATCH v4 15/21] Image: Add network support

2015-09-07 Thread Daniel P. Berrange 
On Fri, Aug 28, 2015 at 01:47:43PM +, Eren Yagdiran wrote:
> Virt-sandbox-image will pass exact network arguments to virt-sandbox
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index c182874..058738a 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -145,9 +145,13 @@ def run(args):
>  if args.connect is not None:
>  cmd.append("-c")
>  cmd.append(args.connect)
> -params = ['-m','host-image:/=%s,format=%s' %(diskfile,format),
> -   '--',
> -   commandToRun]
> +params = ['-m','host-image:/=%s,format=%s' %(diskfile,format)]
> +networkArgs = args.network
> +if networkArgs is not None:
> +params.append('-N')
> +params.append(networkArgs)
> +params.append('--')
> +params.append(commandToRun)
>  cmd = cmd + params
>  subprocess.call(cmd)
>  subprocess.call(["rm", "-rf", diskfile])
> @@ -228,6 +232,8 @@ def gen_run_args(subparser):
>  help=_("Template directory for saving templates"))
>  parser.add_argument("-i","--igniter",
>  help=_("Igniter command for image"))
> +parser.add_argument("-n","--network",
> +help=_("Network params for running template"))

We should use -N here to match virt-sandbox arg syntax, as
-n is used for shorthand of --name, which we also need to
support in virt-sandbox-image


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

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

Re: [libvirt] [sandbox PATCH v4 18/21] Add config for environment variables

2015-09-07 Thread Daniel P. Berrange 
On Fri, Aug 28, 2015 at 01:47:46PM +, Eren Yagdiran wrote:
> Add the config gobject to store custom environment variables.
> This will allow creating custom environment variables on a sandbox
> with a parameter formatted like --env key1=val1
> 
> Add testcase for custom environment variables
> 
> "make check" now includes testcase for environment variables

> diff --git a/libvirt-sandbox/libvirt-sandbox.sym 
> b/libvirt-sandbox/libvirt-sandbox.sym
> index 6f8097a..a09a6c2 100644
> --- a/libvirt-sandbox/libvirt-sandbox.sym
> +++ b/libvirt-sandbox/libvirt-sandbox.sym
> @@ -26,6 +26,12 @@ LIBVIRT_SANDBOX_0.6.0 {
>   gvir_sandbox_config_disk_get_type;
>   gvir_sandbox_config_has_disks;
>  
> + gvir_sandbox_config_add_env;
> + gvir_sandbox_config_add_env_strv;
> + gvir_sandbox_config_add_env_opts;
> + gvir_sandbox_config_env_get_type;
> + gvir_sandbox_config_has_envs;
> +
>   gvir_sandbox_config_mount_add_include;
>   gvir_sandbox_config_mount_get_includes;
>   gvir_sandbox_config_mount_get_target;

This should be added to a new version block 0.6.1, rather than
changing the existing version.


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

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

Re: [libvirt] [sandbox PATCH v4 17/21] Image: man file for virt-sandbox-image

2015-09-07 Thread Daniel P. Berrange 
On Fri, Aug 28, 2015 at 01:47:45PM +, Eren Yagdiran wrote:
> ---
>  bin/Makefile.am|   5 ++
>  bin/virt-sandbox-image.pod | 172 
> +
>  2 files changed, 177 insertions(+)
>  create mode 100644 bin/virt-sandbox-image.pod

This needs to also update libvirt-sandbox.spec.in to add the new
file to the RPM

> diff --git a/bin/virt-sandbox-image.pod b/bin/virt-sandbox-image.pod
> new file mode 100644
> index 000..a85fcd9
> --- /dev/null
> +++ b/bin/virt-sandbox-image.pod

The virt-sandbox-service command split each subcommand up into a separate
man page. We should probably do the same for this too, but that can wait
until after merge.


> +=head1 OPTIONS
> +
> +=over 4
> +
> +=item B template_directory>

Typically the options would be listed before the position
args. eg

  B

Also we should probably use 'templatename' instead of just 'name' to
make things unambiguous


> +=item B -d driver>
> +
> +Run already built image.
> +
> +=over 6
> +
> +=item B
> +
> +Template name to download.
> +
> +=item B
> +
> +Image path where template image will be stored.
> +
> +=item B<-c or --command>
> +
> +Command for running a image. If it is not specified, virt-sandbox-image will 
> try to load command params from specified source. E.g /bin/bash

This is called -i / --ignitor in the code

> +
> +=item B<-n or --network>
> +
> +Network params will be passed directly to the virt-sandbox. More information 
> about network params, See C
> +
> +=item B<-v or --volume>
> +
> +Volume params are for binding host-paths to the guest. E.g -v /home:/home 
> will map /home directory from host to the guest.
> +
> +=item B<-d or --driver>
> +
> +Driver parameter can be specified with only supported driver by 
> libvirt-sandbox. These are lxc:///, qemu:///session, qemu:///system.

And this is -c / --connect

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

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

[libvirt] [PATCH] conf: fix crash when parse a disordered numa settings

2015-09-07 Thread Luyao Huang 
https://bugzilla.redhat.com/show_bug.cgi?id=1260846

Introduced by 8fedbbdb, if we parse the disordered numa
cell, will get segfault. This is because we allow parse
the numa cell not in order and we alloc them before the
parse loop, the cpumask maybe NULL when parse each numa
cell.

Signed-off-by: Luyao Huang 
---
 src/conf/numa_conf.c   | 10 +---
 .../qemuxml2argv-cpu-numa-disordered.xml   | 26 +++
 .../qemuxml2xmlout-cpu-numa-disordered.xml | 29 ++
 tests/qemuxml2xmltest.c|  1 +
 4 files changed, 63 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disordered.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-disordered.xml

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 5c123b9..b5963ac 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -759,11 +759,15 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def,
 }
 VIR_FREE(tmp);
 
-for (j = 0; j < i; j++) {
+for (j = 0; j < n; j++) {
+if (j == cur_cell || !def->mem_nodes[j].cpumask)
+continue;
+
 if (virBitmapOverlaps(def->mem_nodes[j].cpumask,
-  def->mem_nodes[i].cpumask)) {
+  def->mem_nodes[cur_cell].cpumask)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("NUMA cells %zu and %zu have overlapping vCPU 
ids"), i, j);
+   _("NUMA cells %u and %zu have overlapping vCPU 
ids"),
+   cur_cell, j);
 goto cleanup;
 }
 }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disordered.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disordered.xml
new file mode 100644
index 000..ad31607
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disordered.xml
@@ -0,0 +1,26 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  328650
+  328650
+  16
+  
+hvm
+
+  
+  
+
+
+  
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+  /usr/bin/qemu
+  
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-disordered.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-disordered.xml
new file mode 100644
index 000..0a76f12
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-disordered.xml
@@ -0,0 +1,29 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  328650
+  328650
+  16
+  
+hvm
+
+  
+  
+
+
+  
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index d41954e..5a20ebc 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -608,6 +608,7 @@ mymain(void)
 DO_TEST_DIFFERENT("cpu-numa1");
 DO_TEST_DIFFERENT("cpu-numa2");
 DO_TEST_DIFFERENT("cpu-numa-no-memory-element");
+DO_TEST_DIFFERENT("cpu-numa-disordered");
 DO_TEST("cpu-numa-disjoint");
 DO_TEST("cpu-numa-memshared");
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH 0/3] vmx: Add handling for CDROM devices with SCSI passthru and other minor fixes

2015-09-07 Thread Matthias Bolte 
2015-09-02 13:00 GMT+02:00 Michal Privoznik :
> On 01.09.2015 16:52, Matthias Bolte wrote:
>> This patch series addresses this bug report:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1172544
>>
>> It's mean as a replacement for this patch:
>>
>> https://www.redhat.com/archives/libvir-list/2015-August/msg00970.html
>>
>>
>> Matthias Bolte (3):
>>   vmx: Some whitespace cleanup
>>   vmx: The virVMXParseDisk deviceType can be NULL, add some missing
>> checks
>>   vmx: Add handling for CDROM devices with SCSI passthru
>>
>>  src/vmx/vmx.c | 81 ++---
>>  tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.vmx |  6 ++
>>  tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml | 24 +++
>>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.vmx   | 85 
>> +++
>>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml   | 35 ++
>>  tests/vmx2xmltest.c   |  2 +
>>  tests/xml2vmxdata/xml2vmx-cdrom-scsi-passthru.vmx | 14 
>>  tests/xml2vmxdata/xml2vmx-cdrom-scsi-passthru.xml | 14 
>>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-7.vmx   | 25 +++
>>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-7.xml   | 35 ++
>>  tests/xml2vmxtest.c   |  2 +
>>  11 files changed, 296 insertions(+), 27 deletions(-)
>>  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.vmx
>>  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml
>>  create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.vmx
>>  create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml
>>  create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-passthru.vmx
>>  create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-passthru.xml
>>  create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-7.vmx
>>  create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-7.xml
>>
>
> ACK series if you fix the small nit in 2/3.

Fixed and pushed, thanks.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] VMware: Map vpx:// to dcPath

2015-09-07 Thread Richard W.M. Jones 
On Mon, Sep 07, 2015 at 02:29:22PM +0200, Matthias Bolte wrote:
> It's even more complex than that, you can have another level of
> folders in there. The path can have this format for a vpx:// URI:
> 
> [/...]/[/...][/]
> 
> IIRC datacenter and computeresource name cannot contain slashes. At
> leat libvirt assumes that it can't.

Yup, was simplifying for the sake of keeping the email short :-)

> > (2) Can we add some way to more easily map from vpx:// paths to
> > datastore URLs?  The whole process is very complex even without the
> > ambiguity - it takes a couple of pages of code to do the mapping.
> 
> I assume you already connect to the vpx:// URI to get the dsName for a
> given disk image anyway.

Yup, current code is here (which doesn't include connecting to VMware
or getting the domain XML - that is done elsewhere):

https://github.com/libguestfs/libguestfs/blob/master/v2v/input_libvirt_vcenter_https.ml#L175-L258


> I think the datacenter path could be exposed
> as part of the domain XML as
> /path/to/dc similar to
> the way  works. But it would be ignored on parsing.
> 
> Would that work for you? If yes, I can propose a patch that does this.

Absolutely this would be brilliant.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

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

Re: [libvirt] [PATCH 2/3] vmx: The virVMXParseDisk deviceType can be NULL, add some missing checks

2015-09-07 Thread Matthias Bolte 
2015-09-07 8:28 GMT+02:00 Michal Privoznik :
> On 05.09.2015 16:29, Matthias Bolte wrote:
>> 2015-09-02 13:00 GMT+02:00 Michal Privoznik :
>>> On 01.09.2015 16:52, Matthias Bolte wrote:
 ---
  src/vmx/vmx.c | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

 diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
 index ba4d046..9d1574f 100644
 --- a/src/vmx/vmx.c
 +++ b/src/vmx/vmx.c
 @@ -2178,8 +2178,9 @@ virVMXParseDisk(virVMXContext *ctx, 
 virDomainXMLOptionPtr xmlopt, virConfPtr con
  (*def)->transient = STRCASEEQ(mode,

 "independent-nonpersistent");
  } else if (virFileHasSuffix(fileName, ".iso") ||
 -   STRCASEEQ(deviceType, "atapi-cdrom") ||
 -   STRCASEEQ(deviceType, "cdrom-raw")) {
 +   (deviceType &&
 +(STRCASEEQ(deviceType, "atapi-cdrom") ||
 + STRCASEEQ(deviceType, "cdrom-raw" {
  /*
   * This function was called in order to parse a harddisk 
 device,
   * but .iso files, 'atapi-cdrom', and 'cdrom-raw' devices are 
 for
 @@ -2199,13 +2200,12 @@ virVMXParseDisk(virVMXContext *ctx, 
 virDomainXMLOptionPtr xmlopt, virConfPtr con
  if (virFileHasSuffix(fileName, ".iso")) {
  char *tmp;

 -if (deviceType != NULL) {
 -if (STRCASENEQ(deviceType, "cdrom-image")) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _("Expecting VMX entry '%s' to be 
 'cdrom-image' "
 - "but found '%s'"), deviceType_name, 
 deviceType);
 -goto cleanup;
 -}
 +if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _("Expecting VMX entry '%s' to be 
 'cdrom-image' "
 + "but found '%s'"), deviceType_name,
 +   deviceType ? deviceType : "unknown");
>>>
>>> The control can get here iff deviceType != NULL.
>>>
>>
>> The check for NULL was preexisting here, I just merged two nested if
>> blocks here. Also I don't see how you get to the conclusion that the
>> control can only get here if deviceType != NULL. There is no code
>> before or around this block that would do that.
>>
>
> Maybe I should have been more specific. I meant the virReportError().
> There's no need for (preexisting) ternary operator since the if() right
> above checks for deviceType.

Ah, sorry I totally missed that one. I removed it now.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] [PATCH sandbox 1/4] push changing of user ID down into child process

2015-09-07 Thread Cedric Bosdonnat 
On Fri, 2015-09-04 at 12:40 +0100, Daniel P. Berrange wrote:
> When running interactive sandboxes, don't drop privileges in the
> long running libvirt-sandbox-init-common process. This needs to
> be privileged in order to sync, unmount and shutdown the guest
> when the user command is finished. Push changing of user ID into
> the child process, between fork & exec.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-sandbox/libvirt-sandbox-init-common.c | 60 
> +++
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
> b/libvirt-sandbox/libvirt-sandbox-init-common.c
> index d35f760..760509f 100644
> --- a/libvirt-sandbox/libvirt-sandbox-init-common.c
> +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
> @@ -390,30 +390,43 @@ static int change_user(const gchar *user,
>  return 0;
>  }
>  
> -static gboolean run_command(gboolean interactive, gchar **argv,
> +static gboolean run_command(GVirSandboxConfig *config,
>  pid_t *child,
>  int *appin, int *appout, int *apperr)
>  {
> +GVirSandboxConfigInteractive *iconfig = 
> GVIR_SANDBOX_CONFIG_INTERACTIVE(config);
>  int pid;
>  int master = -1;
>  int slave = -1;
>  int pipein[2] = { -1, -1};
>  int pipeout[2] = { -1, -1};
>  int pipeerr[2] = { -1, -1};
> +gchar **appargv = gvir_sandbox_config_get_command(config);
> +gboolean wanttty = gvir_sandbox_config_interactive_get_tty(iconfig);
>  
>  if (debug)
>  fprintf(stderr, "libvirt-sandbox-init-common: running command %s 
> %d\n",
> -argv[0], interactive);
> +appargv[0], wanttty);
>  
>  *appin = *appout = -1;
>  
> -if (interactive) {
> +if (wanttty) {
>  if (openpty(, , NULL, NULL, NULL) < 0) {
>  fprintf(stderr, "Cannot setup slave for child: %s\n",
>  strerror(errno));
>  goto error;
>  }
>  
> +if (!g_str_equal(gvir_sandbox_config_get_username(config), "root")) {
> +if (fchown(slave,
> +   gvir_sandbox_config_get_userid(config),
> +   gvir_sandbox_config_get_groupid(config)) < 0) {
> +fprintf(stderr, "Cannot make PTY available to user: %s\n",
> +strerror(errno));
> +goto error;
> +}
> +}
> +
>  *appin = master;
>  *appout = master;
>  *apperr = master;
> @@ -436,7 +449,13 @@ static gboolean run_command(gboolean interactive, gchar 
> **argv,
>  }
>  
>  if (pid == 0) {
> -if (interactive) {
> +if (change_user(gvir_sandbox_config_get_username(config),
> +gvir_sandbox_config_get_userid(config),
> +gvir_sandbox_config_get_groupid(config),
> +gvir_sandbox_config_get_homedir(config)) < 0)
> +exit(EXIT_FAILURE);
> +
> +if (wanttty) {
>  close(master);
>  close(STDIN_FILENO);
>  close(STDOUT_FILENO);
> @@ -469,24 +488,25 @@ static gboolean run_command(gboolean interactive, gchar 
> **argv,
>  abort();
>  }
>  
> -execv(argv[0], argv);
> -fprintf(stderr, "Cannot execute '%s': %s\n", argv[0], 
> strerror(errno));
> +execv(appargv[0], appargv);
> +fprintf(stderr, "Cannot execute '%s': %s\n", appargv[0], 
> strerror(errno));
>  exit(EXIT_FAILURE);
>  }
>  
> -if (interactive)
> +if (wanttty) {
>  close(slave);
> -else {
> +} else {
>  close(pipein[0]);
>  close(pipeout[1]);
>  close(pipeerr[1]);
>  }
>  
>  *child = pid;
> +g_strfreev(appargv);
>  return TRUE;
>  
>   error:
> -if (interactive) {
> +if (wanttty) {
>  if (master != -1)
>  close(master);
>  if (slave != -1)
> @@ -506,6 +526,7 @@ static gboolean run_command(gboolean interactive, gchar 
> **argv,
>  close(pipeerr[1]);
>  }
>  *appin = *appout = *apperr = -1;
> +g_strfreev(appargv);
>  return FALSE;
>  }
>  
> @@ -639,8 +660,7 @@ typedef enum {
>  GVIR_SANDBOX_CONSOLE_STATE_RUNNING,
>  } GVirSandboxConsoleState;
>  
> -static gboolean eventloop(gboolean interactive,
> -  gchar **appargv,
> +static gboolean eventloop(GVirSandboxConfig *config,
>int sigread,
>int host)
>  {
> @@ -825,8 +845,7 @@ static gboolean eventloop(gboolean interactive,
>  if (rx->buffer[0] == 
> GVIR_SANDBOX_PROTOCOL_HANDSHAKE_SYNC) {
>  if (debug)
>  fprintf(stderr, "Running 
> command\n");
> -if (!run_command(interactive,
>

Re: [libvirt] [PATCH] virsh: Teach attach-interface to --print-xml

2015-09-07 Thread Martin Kletzander 

On Mon, Sep 07, 2015 at 12:16:23PM +0200, Michal Privoznik wrote:

We have the same argument to many other commands that produce an
XML based on what user typed. But unfortunately attach-interface
was missing it. Maybe nobody hasn't needed it yet. Well, I did


"nobody needed it" or maybe "nobody had needed it" if connected with
the previous sentence, but definitely not "nobody hasn't needed it
yet" :)


just now.

Signed-off-by: Michal Privoznik 
---
tools/virsh-domain.c | 20 
tools/virsh.pod  |  4 
2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b029b65..516a51e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -862,6 +862,10 @@ static const vshCmdOptDef opts_attach_interface[] = {
 .type = VSH_OT_BOOL,
 .help = N_("affect current domain")
},
+{.name = "print-xml",
+ .type = VSH_OT_BOOL,
+ .help = N_("print XML document rather than attach the interface")
+},
{.name = NULL}
};

@@ -938,9 +942,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
if (live)
flags |= VIR_DOMAIN_AFFECT_LIVE;

-if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-return false;
-
if (persistent &&
virDomainIsActive(dom) == 1)


You're referencing the domain here, that's not cool when @dom is
uninitialized.  Either move all the logic after the printing or keep
it here making it working only in proper cases (which is probably no
what you want).

Otherwise looks fine.


flags |= VIR_DOMAIN_AFFECT_LIVE;
@@ -1051,6 +1052,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
virBufferAddLit(, "\n");
}

+virBufferAdjustIndent(, -2);
virBufferAddLit(, "\n");

if (virBufferError()) {
@@ -1060,6 +1062,15 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)

xml = virBufferContentAndReset();

+if (vshCommandOptBool(cmd, "print-xml")) {
+vshPrint(ctl, "%s", xml);
+functionReturn = true;
+goto cleanup;
+}
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
if (flags || current)
ret = virDomainAttachDeviceFlags(dom, xml, flags);
else
@@ -1075,7 +1086,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
}

 cleanup:
-virDomainFree(dom);
+if (dom)
+virDomainFree(dom);
virBufferFreeAndReset();
return functionReturn;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 83c445d3..0212e7a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2507,6 +2507,7 @@ Likewise, I<--shareable> is an alias for I<--mode 
shareable>.
[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
[I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
[I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>]
+[I<--print-xml>]

Attach a new network interface to the domain.  I can be
I to indicate connection via a libvirt virtual network, or
@@ -2536,6 +2537,9 @@ kilobytes in a single burst at I speed as described 
in the
Network XML documentation at
L.

+If I<--print-xml> is specified, then the XML of the interface that would be
+attached is printed instead.
+
If I<--live> is specified, affect a running domain.
If I<--config> is specified, affect the next startup of a persistent domain.
If I<--current> is specified, affect the current domain state.
--
2.4.6

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


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

Re: [libvirt] VMware: Map vpx:// to dcPath

2015-09-07 Thread Matthias Bolte 
2015-09-07 12:18 GMT+02:00 Richard W.M. Jones :
>
> vpx:// paths looks like this:
>
>   vpx://vcenter.example.com/MyFolder/MyDatacenter/MyCluster/esxi
>
> but to connect to the datastore to read the underlying disk image,
> libguestfs must form a URL like this:
>
>   
> https://vcenter.example.com/folder/data/guest/guest-flat.vmdk?dcPath=MyFolder/MyDatacenter=datastore
>
> All parts of this URL can be worked out from the URL or the libvirt
> XML *except* the ?dcPath=... parameter.
>
> The problem is that dcPath isn't a straight mapping from the vpx://
> path.  The particular problem is that if there is a cluster name in
> the path (eg 'MyCluster') it appears that we have to remove it.  ie:
>
>   dcPath=MyFolder/MyDatacenter/MyCluster   - does not work
>   dcPath=MyFolder/MyDatacenter - works
>
> That would be OK if there was always a cluster name at the end of the
> path, but there isn't.  Clusters are completely optional, and AFAIK
> you can't tell if something is a cluster path element just by
> examining the name, since clusters can be given arbitrary names by the
> vCenter admin.

It's even more complex than that, you can have another level of
folders in there. The path can have this format for a vpx:// URI:

[/...]/[/...][/]

IIRC datacenter and computeresource name cannot contain slashes. At
leat libvirt assumes that it can't.

You cannot tell what is what in the path just by looking at the path
alone. libvirt handles this by asking the vCenter about this in
esxVI_Context_LookupManagedObjectsByPath.

The path is split at the slashes. Then the first part of the path is
looked up. As long as the part is a folder libvirt continues to look
into that folder for the next part of the path. Once the datacenter
was found this folder lookup logic continues to find a
(cluster-)computeresource. Once the computeresource was found the
hostsystem is looked up in it. If the computeresource is a cluster
then the hostsystem name has to be given in the path. if the
computeresource isn't a cluster then the computeresource and
hostsystem name are identical. Along this lookup libvirt build the
datacenter path and computeresource path and stores it for later use.

> So:
>
> (1) Is there something I'm missing here?  Maybe the libvirt VMware
> driver presents this information already and I'm just missing it?

The information is stored internally in the datacenterPath in the
esxVI_Context, but it is not accessible via libvirt API.

> (2) Can we add some way to more easily map from vpx:// paths to
> datastore URLs?  The whole process is very complex even without the
> ambiguity - it takes a couple of pages of code to do the mapping.

I assume you already connect to the vpx:// URI to get the dsName for a
given disk image anyway. I think the datacenter path could be exposed
as part of the domain XML as
/path/to/dc similar to
the way  works. But it would be ignored on parsing.

Would that work for you? If yes, I can propose a patch that does this.

-- 
Matthias Bolte
http://photron.blogspot.com

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