Re: [libvirt] [PATCH] Return right error code for baselineCPU
On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote:
On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote:
This Python interface code is returning a -1 on errors for the
`baselineCPU' API. Since this API is supposed to return a pointer
the error return value should really be VIR_PY_NONE.
NB: I've checked all the other APIs in this file and this is the
only pointer API that is returning -1.
Signed-off-by: Don Dugger donald.d.dug...@intel.com
---
python/libvirt-override.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index c60747d..b471605 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -4471,13 +4471,13 @@ libvirt_virConnectBaselineCPU(PyObject *self
ATTRIBUTE_UNUSED,
ncpus = PyList_Size(list);
if (VIR_ALLOC_N_QUIET(xmlcpus, ncpus) 0)
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
for (i = 0; i ncpus; i++) {
xmlcpus[i] = PyString_AsString(PyList_GetItem(list, i));
if (xmlcpus[i] == NULL) {
VIR_FREE(xmlcpus);
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
}
}
}
@@ -4489,13 +4489,13 @@ libvirt_virConnectBaselineCPU(PyObject *self
ATTRIBUTE_UNUSED,
VIR_FREE(xmlcpus);
if (base_cpu == NULL)
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
pybase_cpu = PyString_FromString(base_cpu);
VIR_FREE(base_cpu);
if (pybase_cpu == NULL)
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
return pybase_cpu;
}
--
1.7.10.4
ACK. This is correct. But it obviously changes our API so I'm not
really sure how we should handle this, (e.g. document the API as is as
note that its broken or fix it).
I'd say this _also_ depends also on how things are documented. Since
I'm not aware of any documentation explicitly mentioning that this
particular (an only python) API should return -1 in case of allocation
error, I'd say this was an error all along. I'd also say go ahead
with it since this is the proper behavior and I can't imagine a project
where one would rely on this API to return -1 for allocation errors
without a OOM exception instead of all others.
OTOH, though, there are places where similar behavior was already many
times said to be kept for consistency (e.g. public *Free() functions
segfaulting with NULL parameter, even though the HACKING file says
they should be no-op in that case), so I must say this is a thing I
cannot clearly resolve with my own logic. In case of voting, I'm all
for cleaning this 3yo mistake.
My $0.02,
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Pin guest to memory node on NUMA system
On Tue, Nov 19, 2013 at 09:36:49AM -0500, Shivaprasad G Bhat wrote:
Version 2:
Fixed the string formatting errors in v1.
Version 1:
The patch contains the fix for defect 1009880 reported at redhat bugzilla.
Version changes are great to have, but it's good to add them as a
'footnote' or as 'notes' (separated from the commit with three dashes
on a line, i.e. '---\n'). You can also use git-notes(1) and then use
git-format-patch(1) with the '--notes' option, which adds it there
properly.
The root cause is, ever since the subcpusets(vcpu,emulator) were introduced,
the paren cpuset cannot be modified to remove the nodes that are in use by
the subcpusets.
The fix is to break the memory node modification into three steps as to
assign new nodes into the parent first. Change the nodes in the child nodes.
Then remove the old nodes on the parent node.
Please make sure your EDITOR wraps lines, so it's better readable and
it also looks better in git log.
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/qemu/qemu_driver.c | 129
1 file changed, 129 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e8bc04d..2435b75 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8171,7 +8171,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
}
} else if (STREQ(param-field, VIR_DOMAIN_NUMA_NODESET)) {
virBitmapPtr nodeset = NULL;
+virBitmapPtr old_nodeset = NULL;
+virBitmapPtr temp_nodeset = NULL;
char *nodeset_str = NULL;
+char *old_nodeset_str = NULL;
+char *temp_nodeset_str = NULL;
if (virBitmapParse(params[i].value.s,
0, nodeset,
@@ -8181,6 +8185,10 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_LIVE) {
+size_t j;
+virCgroupPtr cgroup_vcpu = NULL;
+virCgroupPtr cgroup_emulator = NULL;
+
if (vm-def-numatune.memory.mode !=
VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -8200,7 +8208,128 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
continue;
}
+if (virCgroupGetCpusetMems(priv-cgroup, old_nodeset_str)
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Failed to get current system nodeset
values));
No need to report second error since virCgroup...() already set the error.
+virBitmapFree(nodeset);
+VIR_FREE(nodeset_str);
+ret = -1;
+continue;
+}
+
+if (virBitmapParse(old_nodeset_str, 0, old_nodeset,
+ VIR_DOMAIN_CPUMASK_LEN) 0) {
+virBitmapFree(nodeset);
+VIR_FREE(nodeset_str);
+VIR_FREE(old_nodeset_str);
+ret = -1;
+continue;
+}
+
+if ((temp_nodeset = virBitmapNewCopy(old_nodeset)) == NULL) {
+virBitmapFree(nodeset);
+VIR_FREE(nodeset_str);
+virBitmapFree(old_nodeset);
+VIR_FREE(old_nodeset_str);
+ret = -1;
+continue;
Just skimming through, I see a lot of unnecessary code duplication
here. Can you break it to different function? Or rework the code to
free/clean the data in one place, since there is more than a few lines
of code in this workpath?
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 0/8] glusterfs storage pool
On 11/25/13 08:43, Osier Yang wrote: On 23/11/13 11:20, Eric Blake wrote: ... Eric Blake (8): storage: initial support for linking with libgfapi storage: document gluster pool storage: implement rudimentary glusterfs pool refresh storage: add network-dir as new storage volume type storage: improve directory support in gluster pool storage: improve allocation stats reported on gluster files storage: improve handling of symlinks in gluster storage: probe qcow2 volumes in gluster pool Looked through the whole set, except the version nit pointed out by Daniel, I didn't see any other problem. So ACK. /btw, I need to support the gluster pool for domain config after these patches are pushed. Don't bother with that at this point. My patches that deal with snaphots on gluster (not yet posted) refactored the code to format pool sources, thus we'd have a ton of rebase conflicts. Regards, Osier Peter 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 v2]lxc: don't do duplicate work when getting pagesize
On 25.11.2013 08:06, Chen Hanxiao wrote:
From: Chen Hanxiao chenhanx...@cn.fujitsu.com
Don't do duplicate work when getting pagesize.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
v2: remove redundant debug log
src/lxc/lxc_container.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 2bdf957..00bbaf7 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -144,6 +144,7 @@ int lxcContainerHasReboot(void)
int cmd, v;
int status;
char *tmp;
+int stacksize = getpagesize() * 4;
if (virFileReadAll(/proc/sys/kernel/ctrl-alt-del, 10, buf) 0)
return -1;
@@ -160,10 +161,10 @@ int lxcContainerHasReboot(void)
VIR_FREE(buf);
cmd = v ? LINUX_REBOOT_CMD_CAD_ON : LINUX_REBOOT_CMD_CAD_OFF;
-if (VIR_ALLOC_N(stack, getpagesize() * 4) 0)
+if (VIR_ALLOC_N(stack, stacksize) 0)
return -1;
-childStack = stack + (getpagesize() * 4);
+childStack = stack + stacksize;
cpid = clone(lxcContainerRebootChild, childStack, flags, cmd);
VIR_FREE(stack);
@@ -2031,6 +2032,7 @@ int lxcContainerStart(virDomainDefPtr def,
/* allocate a stack for the container */
if (VIR_ALLOC_N(stack, stacksize) 0)
return -1;
+
stacktop = stack + stacksize;
cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD;
@@ -2078,6 +2080,7 @@ int lxcContainerAvailable(int features)
int cpid;
char *childStack;
char *stack;
+int stacksize = getpagesize() * 4;
if (features LXC_CONTAINER_FEATURE_USER)
flags |= CLONE_NEWUSER;
@@ -2085,12 +2088,10 @@ int lxcContainerAvailable(int features)
if (features LXC_CONTAINER_FEATURE_NET)
flags |= CLONE_NEWNET;
-if (VIR_ALLOC_N(stack, getpagesize() * 4) 0) {
-VIR_DEBUG(Unable to allocate stack);
+if (VIR_ALLOC_N(stack, stacksize) 0)
return -1;
-}
-childStack = stack + (getpagesize() * 4);
+childStack = stack + stacksize;
cpid = clone(lxcContainerDummyChild, childStack, flags, NULL);
VIR_FREE(stack);
ACKed pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: expose volume meta-type in XML
On Fri, Nov 22, 2013 at 03:26:11PM -0700, Eric Blake wrote: I got annoyed at having to use both 'virsh vol-list $pool --details' AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated the volume correctly. Since two-thirds of the data present in virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(), this just adds the remaining piece of information. * docs/formatstorage.html.in: Document new target type= * docs/schemas/storagevol.rng (target, backingStore): Add it to RelaxNG. * src/conf/storage_conf.h (virStorageVolTypeToString): Declare. * src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output the metatype. * tests/storagevolxml2xmlout/vol-*.xml: Update tests to match. Signed-off-by: Eric Blake ebl...@redhat.com --- diff --git a/tests/storagevolxml2xmlin/vol-logical-backing.xml b/tests/storagevolxml2xmlin/vol-logical-backing.xml index b4141a5..d1a7b61 100644 --- a/tests/storagevolxml2xmlin/vol-logical-backing.xml +++ b/tests/storagevolxml2xmlin/vol-logical-backing.xml @@ -6,6 +6,7 @@ extent start='31440502784' end='33520877568'/ /device /source + typeblock/type capacity2080374784/capacity allocation2080374784/allocation target I think I'd be more inclined to have a top level attribute eg volume type=block ... volume 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] [PATCHv4 4/8] storage: add network-dir as new storage volume type
On Fri, Nov 22, 2013 at 08:20:26PM -0700, Eric Blake wrote:
In the 'directory' and 'netfs' storage pools, a user can see
both 'file' and 'dir' storage volume types, to know when they
can descend into a subdirectory. But in a network-based storage
pool, such as the upcoming 'gluster' pool, we use 'network'
instead of 'file', and did not have any counterpart for a
directory until this patch. Adding a new volume type
'network-dir' is better than reusing 'dir', because it makes
it clear that the only way to access 'network' volumes within
that container is through the network mounting (leaving 'dir'
for something accessible in the local file system).
* include/libvirt/libvirt.h.in (virStorageVolType): Expand enum.
* src/qemu/qemu_command.c (qemuBuildVolumeString): Fix client.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise.
* tools/virsh-volume.c (vshVolumeTypeToString): Likewise.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemVolDelete): Likewise.
Signed-off-by: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 2 ++
src/conf/storage_conf.c | 2 +-
src/qemu/qemu_command.c | 6 --
src/qemu/qemu_conf.c | 4 +++-
src/storage/storage_backend_fs.c | 5 +++--
tools/virsh-volume.c | 5 -
6 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 146a59b..5e8cba6 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2951,6 +2951,8 @@ typedef enum {
VIR_STORAGE_VOL_BLOCK = 1,/* Block based volumes */
VIR_STORAGE_VOL_DIR = 2, /* Directory-passthrough based volume */
VIR_STORAGE_VOL_NETWORK = 3, /* Network volumes like RBD (RADOS Block
Device) */
+VIR_STORAGE_VOL_NETWORK_DIR = 4, /* Network accessible directory that can
+ * contain other network volumes */
#ifdef VIR_ENUM_SENTINELS
VIR_STORAGE_VOL_LAST
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index add2ae1..493a874 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -53,7 +53,7 @@
VIR_ENUM_IMPL(virStorageVol,
VIR_STORAGE_VOL_LAST,
- file, block, dir, network)
+ file, block, dir, network, network-dir)
I've got to say I really don't like this naming but not
got a better suggestion yet.
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] Return right error code for baselineCPU
On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote:
On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote:
This Python interface code is returning a -1 on errors for the
`baselineCPU' API. Since this API is supposed to return a pointer
the error return value should really be VIR_PY_NONE.
NB: I've checked all the other APIs in this file and this is the
only pointer API that is returning -1.
Signed-off-by: Don Dugger donald.d.dug...@intel.com
---
python/libvirt-override.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index c60747d..b471605 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -4471,13 +4471,13 @@ libvirt_virConnectBaselineCPU(PyObject *self
ATTRIBUTE_UNUSED,
ncpus = PyList_Size(list);
if (VIR_ALLOC_N_QUIET(xmlcpus, ncpus) 0)
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
for (i = 0; i ncpus; i++) {
xmlcpus[i] = PyString_AsString(PyList_GetItem(list, i));
if (xmlcpus[i] == NULL) {
VIR_FREE(xmlcpus);
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
}
}
}
@@ -4489,13 +4489,13 @@ libvirt_virConnectBaselineCPU(PyObject *self
ATTRIBUTE_UNUSED,
VIR_FREE(xmlcpus);
if (base_cpu == NULL)
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
pybase_cpu = PyString_FromString(base_cpu);
VIR_FREE(base_cpu);
if (pybase_cpu == NULL)
-return VIR_PY_INT_FAIL;
+return VIR_PY_NONE;
return pybase_cpu;
}
--
1.7.10.4
ACK. This is correct. But it obviously changes our API so I'm not
really sure how we should handle this, (e.g. document the API as is as
note that its broken or fix it).
The implicit expectation with python APIs is that they all raise an
exception if the libvirt call fails. So ACK to this bug fix we
should put it in maint branches.
That said, please hold off applying this to GIT. We'll put it into
the new libvirt-python git I'm about to push instead.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: add securetty related note in Device nodes
On Mon, Nov 25, 2013 at 02:47:53PM +0800, Gao feng wrote: Tell user how to resolve the problem that fail to log in the container. Signed-off-by: Gao feng gaof...@cn.fujitsu.com --- docs/drvlxc.html.in | 8 1 file changed, 8 insertions(+) diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in index ca488f7..d5a4410 100644 --- a/docs/drvlxc.html.in +++ b/docs/drvlxc.html.in @@ -164,6 +164,14 @@ numbered incrementally from there. /p p +Since /dev/ttyN and /dev/console are linked to the pts devices. The +tty device of login program is pts device. the pam module securetty +may prevent root user from logging in container. If you want root +user to log in container successfully, add the pts device to the file +/etc/securetty of container. +/p + +p Further block or character devices will be made available to containers depending on their configuration. /p 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 1/4] vbox: cleanup vboxAttachUSB
On 11/21/2013 04:41 PM, Ryota Ozaki wrote:
This cleanup flattens deeply nested code.
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
ACK and pushed. Fairly mechanical. (Still concerned by the lack of error
checking, but that is a pre-existing problem...)
---
src/vbox/vbox_tmpl.c | 153
++-
1 file changed, 79 insertions(+), 74 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 942570f..67dd23a 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -4852,94 +4852,99 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData
*data, IMachine *machine)
* usual
*/
for (i = 0; i def-nhostdevs; i++) {
-if (def-hostdevs[i]-mode ==
-VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
-if (def-hostdevs[i]-source.subsys.type ==
-VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
-if (def-hostdevs[i]-source.subsys.u.usb.vendor ||
-def-hostdevs[i]-source.subsys.u.usb.product) {
-VIR_DEBUG(USB Device detected, VendorId:0x%x,
ProductId:0x%x,
- def-hostdevs[i]-source.subsys.u.usb.vendor,
- def-hostdevs[i]-source.subsys.u.usb.product);
-isUSB = true;
-break;
-}
-}
-}
+if (def-hostdevs[i]-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+continue;
+
+if (def-hostdevs[i]-source.subsys.type !=
+VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+continue;
+
+if (!def-hostdevs[i]-source.subsys.u.usb.vendor
+!def-hostdevs[i]-source.subsys.u.usb.product)
+continue;
+
+VIR_DEBUG(USB Device detected, VendorId:0x%x, ProductId:0x%x,
+ def-hostdevs[i]-source.subsys.u.usb.vendor,
+ def-hostdevs[i]-source.subsys.u.usb.product);
+isUSB = true;
+break;
}
-if (isUSB) {
-/* First Start the USB Controller and then loop
- * to attach USB Devices to it
- */
-machine-vtbl-GetUSBController(machine, USBController);
-if (USBController) {
-USBController-vtbl-SetEnabled(USBController, 1);
+if (!isUSB)
+return;
+
+/* First Start the USB Controller and then loop
+ * to attach USB Devices to it
+ */
+machine-vtbl-GetUSBController(machine, USBController);
+
+if (!USBController)
+return;
+
+USBController-vtbl-SetEnabled(USBController, 1);
#if VBOX_API_VERSION 4002
-USBController-vtbl-SetEnabledEhci(USBController, 1);
+USBController-vtbl-SetEnabledEhci(USBController, 1);
#else
-USBController-vtbl-SetEnabledEHCI(USBController, 1);
+USBController-vtbl-SetEnabledEHCI(USBController, 1);
#endif
-for (i = 0; i def-nhostdevs; i++) {
-if (def-hostdevs[i]-mode ==
-VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
-if (def-hostdevs[i]-source.subsys.type ==
-VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
+for (i = 0; i def-nhostdevs; i++) {
+char *filtername = NULL;
+PRUnichar *filternameUtf16 = NULL;
+IUSBDeviceFilter *filter = NULL;
+PRUnichar *vendorIdUtf16 = NULL;
+char vendorId[40] = {0};
+PRUnichar *productIdUtf16 = NULL;
+char productId[40]= {0};
-char *filtername = NULL;
-PRUnichar *filternameUtf16 = NULL;
-IUSBDeviceFilter *filter = NULL;
+if (def-hostdevs[i]-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+continue;
-/* Zero pad for nice alignment when fewer than
- * devices.
- */
-if (virAsprintf(filtername, filter%04zu, i) = 0)
{
-VBOX_UTF8_TO_UTF16(filtername, filternameUtf16);
-VIR_FREE(filtername);
-
USBController-vtbl-CreateDeviceFilter(USBController,
-
filternameUtf16,
-filter);
-}
-VBOX_UTF16_FREE(filternameUtf16);
+if (def-hostdevs[i]-source.subsys.type !=
+VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+continue;
-if (filter
-(def-hostdevs[i]-source.subsys.u.usb.vendor ||
- def-hostdevs[i]-source.subsys.u.usb.product))
{
+/* Zero pad for nice alignment when fewer than
+ * devices.
+ */
+if (virAsprintf(filtername,
Re: [libvirt] [PATCH 4/4] vbox: add support for 4.3 APIs
On 11/21/2013 04:41 PM, Ryota Ozaki wrote: Makefile.am, vbox_V4_3.c and vbox_driver.c do regular modifitions to support a new version of APIs. vbox_tmpl.c basically fixes incompatibilities since 4.2. The affected incompatibilities of 4.3 are: * IMachine::Delete() has been renamed to IMachine::deleteConfig() * IMedium::CreateBaseStorage() now accepts multiple variant values * IDisplay::GetScreenResolution() now returns the display position in the guest * IMachine now has multiple IUSBControllers and IUSBDeviceFilters handles USB device filters instead of (obsolete) IUSBController This patch is tested on Mac OS X 10.8.5 and Fedora 19. Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com --- src/Makefile.am| 3 +- src/vbox/vbox_V4_3.c | 13 + src/vbox/vbox_driver.c | 8 ++ src/vbox/vbox_tmpl.c | 74 -- 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 src/vbox/vbox_V4_3.c I won't pretend to understand anything about the differences between vbox versions. But the chnages inserted seem sane, and it passes a make check make syntax-check make rpm with vbox support enabled, so ACK and pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] vbox: pull vboxHostDeviceGetXMLDesc out from vboxDomainGetXMLDesc
On 11/21/2013 04:41 PM, Ryota Ozaki wrote:
The USB-related code in vboxDomainGetXMLDesc is deeply nested and
difficult to add new code. So flatten it. To do so, the code is
pulled out from vboxDomainGetXMLDesc to make the function short
and to leaverage early return and goto for error handling.
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
---
src/vbox/vbox_tmpl.c | 183
+++
1 file changed, 97 insertions(+), 86 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 67dd23a..95a04b1 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2206,6 +2206,102 @@ vboxDomainGetMaxVcpus(virDomainPtr dom)
VIR_DOMAIN_VCPU_MAXIMUM));
}
+static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr
def, IMachine *machine)
+{
+IUSBController *USBController = NULL;
+PRBool enabled = PR_FALSE;
+vboxArray deviceFilters = VBOX_ARRAY_INITIALIZER;
+size_t i;
+PRUint32 USBFilterCount = 0;
+
+def-nhostdevs = 0;
+machine-vtbl-GetUSBController(machine, USBController);
+
+if (!USBController)
+return;
+
+USBController-vtbl-GetEnabled(USBController, enabled);
+if (!enabled)
+goto release_controller;
+
+vboxArrayGet(deviceFilters, USBController,
+ USBController-vtbl-GetDeviceFilters);
+
+if (deviceFilters.count = 0)
+goto release_filters;
+
+/* check if the filters are active and then only
+ * alloc mem and set def-nhostdevs
+ */
+
+for (i = 0; i deviceFilters.count; i++) {
+PRBool active = PR_FALSE;
+IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
+
+deviceFilter-vtbl-GetActive(deviceFilter, active);
+if (active) {
+def-nhostdevs++;
+}
+}
+
+if (def-nhostdevs == 0)
+goto release_filters;
+
+/* Alloc mem needed for the filters now */
+if (VIR_ALLOC_N(def-hostdevs, def-nhostdevs) 0)
+goto release_filters;
+
+for (i = 0; (USBFilterCount def-nhostdevs) || (i
deviceFilters.count); i++) {
+PRBool active = PR_FALSE;
+IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
+PRUnichar *vendorIdUtf16 = NULL;
+char *vendorIdUtf8 = NULL;
+unsigned vendorId = 0;
+PRUnichar *productIdUtf16 = NULL;
+char *productIdUtf8= NULL;
+unsigned productId = 0;
+char *endptr = NULL;
Again, very mechanical. ACK and pushed.
I just want to make sure of one thing though - there are several char*'s
here that are set by calling some other function unknown to me, so I
don't know if those functions are returning a pointer to a pre-existing
string, or allocating a new string. (The standard practice in libvirt
code is to declare things that are simply pointing to pre-existing
buffers as const char * to avoid confusion). Can you verify that we're
not leaking anything here? (Again, this is something that you didn't
change, so has no bearing on whether or not the current patch should go in).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] vbox: import vbox_CAPI_v4_3.h from SDK
On 11/21/2013 04:41 PM, Ryota Ozaki wrote: vbox_CAPI_v4_3.h is almost same as sdk/bindings/xpcom/include/VBoxCAPI_v4_3.h of http://download.virtualbox.org/virtualbox/4.3.2/VirtualBoxSDK-4.3.2-90405.zip, but modified to fix preprocessor indentations by using cppi. Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com --- src/vbox/vbox_CAPI_v4_3.h | 10210 1 file changed, 10210 insertions(+) create mode 100644 src/vbox/vbox_CAPI_v4_3.h Sigh. I see from the existing code in the vbox directory that importing the entire (very long)( header file (with small tweaks such as proper indentation for cppi) is standard practice, so ACK to this. But I can't let it go by without asking - is there really no standard set of header files for vbox development that can/should be distributed separately? Note the one correction below, though. diff --git a/src/vbox/vbox_CAPI_v4_3.h b/src/vbox/vbox_CAPI_v4_3.h new file mode 100644 index 000..92d810b --- /dev/null +++ b/src/vbox/vbox_CAPI_v4_3.h @@ -0,0 +1,10210 @@ +/* + * Libvirt notice: this file is derived from the VirtualBox SDK, with + * libvirt edits (fixing preprocessor indentation by cppi); do not + * regenerate in the context of libvirt. + */ +/* + * DO NOT EDIT! This is a generated file. + * + * XPCOM IDL (XPIDL) definition for VirtualBox Main API (COM interfaces) + * generated from XIDL (XML interface definition). + * + * Source: src/VBox/Main/idl/VirtualBox.xidl + * Generator : src/VBox/Main/idl/xpcidl.xsl + * + * This file contains portions from the following Mozilla XPCOM files: + * xpcom/include/xpcom/nsID.h + * xpcom/include/nsIException.h + * xpcom/include/nsprpub/prtypes.h + * xpcom/include/xpcom/nsISupportsBase.h + * + * These files were originally triple-licensed (MPL/GPL2/LGPL2.1). Oracle + * elects to distribute this derived work under the LGPL2.1 only. + */ + +/* + * Copyright (C) 2008-2012 Oracle Corporation + * + * This file is part of a free software library; you can redistribute + * it and/or modify it under the terms of the GNU Lesser General + * Public License version 2.1 as published by the Free Software + * Foundation and shipped in the COPYING file with this library. Since the file is released under LGPL, it should refer to the file COPYING.LESSER instead. I've made that change and pushed it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: fix double articles bug
Delete the extra article 'the'. Signed-off-by: Wang Yufei james.wangyu...@huawei.com --- src/libvirt.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index eff44eb..0eb4c64 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16525,7 +16525,7 @@ error: * Otherwise, creates a new secret with an automatically chosen UUID, and * initializes its attributes from xml. * - * Returns a the secret on success, NULL on failure. + * Returns a secret on success, NULL on failure. */ virSecretPtr virSecretDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) -- 1.7.3.1.msysgit.0 Best Regards, -WangYufei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv6 1/5] Add a hostdev PCI backend type
On 11/22/2013 11:08 PM, Jim Fehlig wrote: Chunyan Liu wrote: Add VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN. For legacy xen, it will use pciback as stub driver. Sorry for the long delay in helping review these patches. I finally have some time to work on libvirt :). With the pending release, this series will have to wait for the next cycle, but hopefully we can get it in early for prolonged testing. It plugs a big hole in the libxl dirver, so thanks for your work and perseverance :). I've applied your patches to latest git master and testing looks good so far, after fixing a small issue in 2/5. See the individual patches for further comments. The part that has made me nervous about this series from the beginning is that it is moving the implementation of some pretty hairy code from qemu-specific to a general library while that code has itself been getting fairly frequently bugfix tweaks. Fortunately we now have several more unit tests on this code, but before pushing we still should do a bit of extra due diligence to make sure that no recent bugfixes to the qemu-specific version of the code are missing in the new versions. (BTW, for this reason I think it is a very good idea to switch qemu and lxc over to this new library immediately, rather than waiting as was done in an earlier version of the patch series.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] vbox: pull vboxHostDeviceGetXMLDesc out from vboxDomainGetXMLDesc
Hi Laine,
On Mon, Nov 25, 2013 at 8:26 PM, Laine Stump la...@laine.org wrote:
On 11/21/2013 04:41 PM, Ryota Ozaki wrote:
The USB-related code in vboxDomainGetXMLDesc is deeply nested and
difficult to add new code. So flatten it. To do so, the code is
pulled out from vboxDomainGetXMLDesc to make the function short
and to leaverage early return and goto for error handling.
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
---
src/vbox/vbox_tmpl.c | 183
+++
1 file changed, 97 insertions(+), 86 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 67dd23a..95a04b1 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2206,6 +2206,102 @@ vboxDomainGetMaxVcpus(virDomainPtr dom)
VIR_DOMAIN_VCPU_MAXIMUM));
}
+static void vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr
def, IMachine *machine)
+{
+IUSBController *USBController = NULL;
+PRBool enabled = PR_FALSE;
+vboxArray deviceFilters = VBOX_ARRAY_INITIALIZER;
+size_t i;
+PRUint32 USBFilterCount = 0;
+
+def-nhostdevs = 0;
+machine-vtbl-GetUSBController(machine, USBController);
+
+if (!USBController)
+return;
+
+USBController-vtbl-GetEnabled(USBController, enabled);
+if (!enabled)
+goto release_controller;
+
+vboxArrayGet(deviceFilters, USBController,
+ USBController-vtbl-GetDeviceFilters);
+
+if (deviceFilters.count = 0)
+goto release_filters;
+
+/* check if the filters are active and then only
+ * alloc mem and set def-nhostdevs
+ */
+
+for (i = 0; i deviceFilters.count; i++) {
+PRBool active = PR_FALSE;
+IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
+
+deviceFilter-vtbl-GetActive(deviceFilter, active);
+if (active) {
+def-nhostdevs++;
+}
+}
+
+if (def-nhostdevs == 0)
+goto release_filters;
+
+/* Alloc mem needed for the filters now */
+if (VIR_ALLOC_N(def-hostdevs, def-nhostdevs) 0)
+goto release_filters;
+
+for (i = 0; (USBFilterCount def-nhostdevs) || (i
deviceFilters.count); i++) {
+PRBool active = PR_FALSE;
+IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];
+PRUnichar *vendorIdUtf16 = NULL;
+char *vendorIdUtf8 = NULL;
+unsigned vendorId = 0;
+PRUnichar *productIdUtf16 = NULL;
+char *productIdUtf8= NULL;
+unsigned productId = 0;
+char *endptr = NULL;
Again, very mechanical. ACK and pushed.
I just want to make sure of one thing though - there are several char*'s
here that are set by calling some other function unknown to me, so I
don't know if those functions are returning a pointer to a pre-existing
string, or allocating a new string. (The standard practice in libvirt
code is to declare things that are simply pointing to pre-existing
buffers as const char * to avoid confusion). Can you verify that we're
not leaking anything here? (Again, this is something that you didn't
change, so has no bearing on whether or not the current patch should go in).
I looked more and confirmed that the below code
allocates and frees strings correctly.
+deviceFilter-vtbl-GetVendorId(deviceFilter, vendorIdUtf16);
+deviceFilter-vtbl-GetProductId(deviceFilter, productIdUtf16);
+
+VBOX_UTF16_TO_UTF8(vendorIdUtf16, vendorIdUtf8);
+VBOX_UTF16_TO_UTF8(productIdUtf16, productIdUtf8);
+
(snip)
+
+VBOX_UTF16_FREE(vendorIdUtf16);
+VBOX_UTF8_FREE(vendorIdUtf8);
+
+VBOX_UTF16_FREE(productIdUtf16);
+VBOX_UTF8_FREE(productIdUtf8);
VBOX_UTF16_TO_UTF8 is a wrapper of pfnUtf16ToUtf8()
and VBOX_UTF{16,8}_FREE are pfnUtf{16,8}Free().
And this is a code snippet from SDK:
PRUnichar *uuidUtf16 = NULL;
char *uuidUtf8 = NULL;
machine-vtbl-GetId(machine, uuidUtf16);
g_pVBoxFuncs-pfnUtf16ToUtf8(uuidUtf16, uuidUtf8);
printf(\tUUID:%s\n, uuidUtf8);
g_pVBoxFuncs-pfnUtf8Free(uuidUtf8);
g_pVBoxFuncs-pfnUtf16Free(uuidUtf16);
So the usage of the functions in vbox_tmpl.c looks ok for me.
Thanks,
ozaki-r
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Make pci config files writable
As of 21685c955 the 'distcheck' is broken. The problem is, by default it copies all the necessary files and make them read only. However, pci device detach test doesn't work that way. The PCI device configs are stored within our tests/ directory and need to be writable (detaching and resetting means writing into the config file). Hence, we must make those files writable again. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/Makefile.am | 5 + 1 file changed, 5 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index e46d5f7..29dbf76 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -921,3 +921,8 @@ endif ! WITH_CIL CLEANFILES = *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.cmi *.cmx \ object-locking-files.txt + +# Some tests tend to write into files. Notably, the virpcitest, which detach +# and reset a pci device (achieved byt writing into a pci config file). +check-local: + chmod -R u+w $(srcdir)/virpcitestdata/ -- 1.8.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Make pci config files writable
On Mon, Nov 25, 2013 at 03:22:38PM +0100, Michal Privoznik wrote: As of 21685c955 the 'distcheck' is broken. The problem is, by default it copies all the necessary files and make them read only. However, pci device detach test doesn't work that way. The PCI device configs are stored within our tests/ directory and need to be writable (detaching and resetting means writing into the config file). Hence, we must make those files writable again. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/Makefile.am | 5 + 1 file changed, 5 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index e46d5f7..29dbf76 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -921,3 +921,8 @@ endif ! WITH_CIL CLEANFILES = *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.cmi *.cmx \ object-locking-files.txt + +# Some tests tend to write into files. Notably, the virpcitest, which detach +# and reset a pci device (achieved byt writing into a pci config file). +check-local: + chmod -R u+w $(srcdir)/virpcitestdata/ I seem to recall eric saying on IRC that we shouldn't do this, and that any files the test suite writes to should be located in $(builddir) not $(srcdir). 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] maint: next release is 1.2.0
I didn't find any other instances with:
git grep '1\.1\.5'
* src/test/test_driver.c (testDriver): Tweak version info.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Pushing under the trivial rule.
src/test/test_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 469223e..c1f1cd4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7193,7 +7193,7 @@ static virDriver testDriver = {
.domainRevertToSnapshot = testDomainRevertToSnapshot, /* 1.1.4 */
.domainSnapshotDelete = testDomainSnapshotDelete, /* 1.1.4 */
-.connectBaselineCPU = testConnectBaselineCPU, /* 1.1.5 */
+.connectBaselineCPU = testConnectBaselineCPU, /* 1.2.0 */
};
static virNetworkDriver testNetworkDriver = {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] vbox: import vbox_CAPI_v4_3.h from SDK
On Mon, Nov 25, 2013 at 8:26 PM, Laine Stump la...@laine.org wrote: On 11/21/2013 04:41 PM, Ryota Ozaki wrote: vbox_CAPI_v4_3.h is almost same as sdk/bindings/xpcom/include/VBoxCAPI_v4_3.h of http://download.virtualbox.org/virtualbox/4.3.2/VirtualBoxSDK-4.3.2-90405.zip, but modified to fix preprocessor indentations by using cppi. Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com --- src/vbox/vbox_CAPI_v4_3.h | 10210 1 file changed, 10210 insertions(+) create mode 100644 src/vbox/vbox_CAPI_v4_3.h Sigh. I see from the existing code in the vbox directory that importing the entire (very long)( header file (with small tweaks such as proper indentation for cppi) is standard practice, so ACK to this. But I can't let it go by without asking - is there really no standard set of header files for vbox development that can/should be distributed separately? Yes, unfortunately :-/ Note the one correction below, though. diff --git a/src/vbox/vbox_CAPI_v4_3.h b/src/vbox/vbox_CAPI_v4_3.h new file mode 100644 index 000..92d810b --- /dev/null +++ b/src/vbox/vbox_CAPI_v4_3.h @@ -0,0 +1,10210 @@ +/* + * Libvirt notice: this file is derived from the VirtualBox SDK, with + * libvirt edits (fixing preprocessor indentation by cppi); do not + * regenerate in the context of libvirt. + */ +/* + * DO NOT EDIT! This is a generated file. + * + * XPCOM IDL (XPIDL) definition for VirtualBox Main API (COM interfaces) + * generated from XIDL (XML interface definition). + * + * Source: src/VBox/Main/idl/VirtualBox.xidl + * Generator : src/VBox/Main/idl/xpcidl.xsl + * + * This file contains portions from the following Mozilla XPCOM files: + * xpcom/include/xpcom/nsID.h + * xpcom/include/nsIException.h + * xpcom/include/nsprpub/prtypes.h + * xpcom/include/xpcom/nsISupportsBase.h + * + * These files were originally triple-licensed (MPL/GPL2/LGPL2.1). Oracle + * elects to distribute this derived work under the LGPL2.1 only. + */ + +/* + * Copyright (C) 2008-2012 Oracle Corporation + * + * This file is part of a free software library; you can redistribute + * it and/or modify it under the terms of the GNU Lesser General + * Public License version 2.1 as published by the Free Software + * Foundation and shipped in the COPYING file with this library. Since the file is released under LGPL, it should refer to the file COPYING.LESSER instead. I've made that change and pushed it. Thanks for the tweak! (hmm, their SDK doesn't include even COPYING file...) ozaki-r -- 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 4/4] vbox: add support for 4.3 APIs
On Mon, Nov 25, 2013 at 8:26 PM, Laine Stump la...@laine.org wrote: On 11/21/2013 04:41 PM, Ryota Ozaki wrote: Makefile.am, vbox_V4_3.c and vbox_driver.c do regular modifitions to support a new version of APIs. vbox_tmpl.c basically fixes incompatibilities since 4.2. The affected incompatibilities of 4.3 are: * IMachine::Delete() has been renamed to IMachine::deleteConfig() * IMedium::CreateBaseStorage() now accepts multiple variant values * IDisplay::GetScreenResolution() now returns the display position in the guest * IMachine now has multiple IUSBControllers and IUSBDeviceFilters handles USB device filters instead of (obsolete) IUSBController This patch is tested on Mac OS X 10.8.5 and Fedora 19. Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com --- src/Makefile.am| 3 +- src/vbox/vbox_V4_3.c | 13 + src/vbox/vbox_driver.c | 8 ++ src/vbox/vbox_tmpl.c | 74 -- 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 src/vbox/vbox_V4_3.c I won't pretend to understand anything about the differences between vbox versions. But the chnages inserted seem sane, and it passes a make check make syntax-check make rpm with vbox support enabled, so ACK and pushed. Thanks a lot for your reviewing and pushing! ozaki-r -- 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] [PATCHv2] storage: allow interleave in volume XML
On 11/24/2013 09:24 PM, Osier Yang wrote: On 23/11/13 03:54, Eric Blake wrote: The RNG grammar did not allow arbitrary interleaving, which makes it harder than necessary to create a new volume from handwritten XML. (Compare also to commit caf516db for pools). * docs/schemas/storagevol.rng: Support interleaving. * tests/storagevolxml2xmlin/vol-file-backing.xml: Test it. Signed-off-by: Eric Blake ebl...@redhat.com --- ACK Thanks; pushed. -- 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] Remove python binding
On Fri, Nov 22, 2013 at 12:08:46PM -0700, Eric Blake wrote:
FYI I ran 'autobuild.sh' to validate the full RPM builds here.
All right, looks like we're nearly ready to pull the trigger then :)
So here's what I propose changing vs my patch
diff --git a/configure.ac b/configure.ac
index 044cf37..a021fcf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2012,7 +2012,8 @@ fi
AM_CONDITIONAL([WITH_HYPERV], [test $with_hyperv = yes])
-dnl Allow perl overrides
+dnl Allow perl/python overrides
+AC_PATH_PROG([PYTHON], [python])
AC_PATH_PROG([PERL], [perl])
AC_ARG_ENABLE([with-test-suite],
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 5bc53a0..0c33608 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -424,6 +424,7 @@ BuildRequires: gettext-devel
BuildRequires: libtool
BuildRequires: /usr/bin/pod2man
%endif
+BuildRequires: python
%if %{with_systemd}
BuildRequires: systemd-units
%endif
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] New libvirt python GIT repository
We have now pushed the python binding code to its new GIT repository location. If you intend to work on python bindings, please get yourself a checkout of the following http://libvirt.org/git/?p=libvirt-python.git;a=summary git://libvirt.org/libvirt-python.git The python binding will generally be released at the same time as major libvirt releases, if new manually written bindings were required. If all new APIs were correctly handled by the code generator, we will not need to do a release. The minor maint branches will also be separate for libvirt.git vs libvirt-python.git. It is unlikely we'll need to do maint branches for the python code most of the time. If we do though, there is no need to synchronize with libvirt maint releases. 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] tests: Make pci config files writable
On 11/25/2013 07:24 AM, Daniel P. Berrange wrote: object-locking-files.txt + +# Some tests tend to write into files. Notably, the virpcitest, which detach +# and reset a pci device (achieved byt writing into a pci config file). +check-local: +chmod -R u+w $(srcdir)/virpcitestdata/ NACK. This will fail if $(srcdir) is on a read-only mount point (such as a CDROM). I seem to recall eric saying on IRC that we shouldn't do this, and that any files the test suite writes to should be located in $(builddir) not $(srcdir). Indeed. The correct way is to move these files to $(builddir) (which is always writable) and quit trying to ship them in the tarball - they only need to exist for the duration of the test, so the tarball only needs rules on how to generate the files into $(builddir) at the start of the test. -- 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] Remove python binding
On 11/25/2013 08:09 AM, Daniel P. Berrange wrote: On Fri, Nov 22, 2013 at 12:08:46PM -0700, Eric Blake wrote: FYI I ran 'autobuild.sh' to validate the full RPM builds here. All right, looks like we're nearly ready to pull the trigger then :) So here's what I propose changing vs my patch ACK. Let's do it! -- 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] Return right error code for baselineCPU
On Mon, Nov 25, 2013 at 10:45:38AM +, Daniel P. Berrange wrote: On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote: On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote: This Python interface code is returning a -1 on errors for the `baselineCPU' API. Since this API is supposed to return a pointer the error return value should really be VIR_PY_NONE. ... ACK. This is correct. But it obviously changes our API so I'm not really sure how we should handle this, (e.g. document the API as is as note that its broken or fix it). The implicit expectation with python APIs is that they all raise an exception if the libvirt call fails. So ACK to this bug fix we should put it in maint branches. Much as I hate to raise the issue this assumption is true for pointer APIs but APIs that return an integer don't raise an exception, they just return -1. Obviously, changing this behavior would be way too invasive but documenting this behavior should be done somewhere. -- 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
Re: [libvirt] [PATCHv4 1/8] storage: initial support for linking with libgfapi
On Fri, Nov 22, 2013 at 08:20:23PM -0700, Eric Blake wrote: We support gluster volumes in domain XML, so we also ought to support them as a storage pool. Besides, a future patch will want to take advantage of libgfapi to handle the case of a gluster device holding qcow2 rather than raw storage, and for that to work, we need a storage backend that can read gluster storage volume contents. This sets up the framework. Note that the new pool is named 'gluster' to match a disk type='network'source protocol='gluster' image source already supported in a domain; it does NOT match the pool type='netfs'sourcetarget type='glusterfs', since that uses a FUSE mount to a local file name rather than a network name. This and subsequent patches have been tested against glusterfs 3.4.1 (available on Fedora 19); there are likely bugs in older versions that may prevent decent use of gfapi, so this patch enforces the minimum version tested. A future patch may lower the minimum. On the other hand, I hit at least two bugs in 3.4.1 that will be fixed in 3.5/3.4.2, where it might be worth raising the minimum: glfs_readdir is nicer to use than glfs_readdir_r [1], and glfs_fini should only return failure on an actual failure [2]. [1] http://lists.gnu.org/archive/html/gluster-devel/2013-10/msg00085.html [2] http://lists.gnu.org/archive/html/gluster-devel/2013-10/msg00086.html * configure.ac (WITH_STORAGE_GLUSTER): New conditional. * m4/virt-gluster.m4: new file. * libvirt.spec.in (BuildRequires): Support gluster in spec file. * src/conf/storage_conf.h (VIR_STORAGE_POOL_GLUSTER): New pool type. * src/conf/storage_conf.c (poolTypeInfo): Treat similar to sheepdog and rbd. (virStoragePoolDefFormat): Don't output target for gluster. * src/storage/storage_backend_gluster.h: New file. * src/storage/storage_backend_gluster.c: Likewise. * po/POTFILES.in: Add new file. * src/storage/storage_backend.c (backends): Register new type. * src/Makefile.am (STORAGE_DRIVER_GLUSTER_SOURCES): Build new files. * src/storage/storage_backend.h (_virStorageBackend): Documet assumption. Signed-off-by: Eric Blake ebl...@redhat.com --- configure.ac | 21 libvirt.spec.in | 15 m4/virt-gluster.m4| 28 + po/POTFILES.in| 1 + src/Makefile.am | 10 src/conf/storage_conf.c | 26 +--- src/conf/storage_conf.h | 3 ++- src/storage/storage_backend.c | 6 + src/storage/storage_backend.h | 6 +++-- src/storage/storage_backend_gluster.c | 46 +++ src/storage/storage_backend_gluster.h | 29 ++ 11 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 m4/virt-gluster.m4 create mode 100644 src/storage/storage_backend_gluster.c create mode 100644 src/storage/storage_backend_gluster.h 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 v4] virsh domxml-from-native to treat SCSI as the bus type for pseries by default
On 11/22/2013 12:27 PM, Shivaprasad G Bhat wrote: The bus type IDE being enum Zero, the bus type on pseries system appears as IDE for all the -hda/-cdrom and for disk drives with if=none type. Pseries platform needs this to appear as SCSI instead of IDE. The ide being not supported, the explicit requests for ide devices will return an error. Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com --- src/qemu/qemu_command.c| 25 +++-- tests/qemuargv2xmltest.c |1 + .../qemuxml2argv-pseries-disk.args |5 +++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 40 4 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml ACK, pushed now. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Return right error code for baselineCPU
On Mon, Nov 25, 2013 at 08:40:41AM -0700, Don Dugger wrote: On Mon, Nov 25, 2013 at 10:45:38AM +, Daniel P. Berrange wrote: On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote: On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote: This Python interface code is returning a -1 on errors for the `baselineCPU' API. Since this API is supposed to return a pointer the error return value should really be VIR_PY_NONE. ... ACK. This is correct. But it obviously changes our API so I'm not really sure how we should handle this, (e.g. document the API as is as note that its broken or fix it). The implicit expectation with python APIs is that they all raise an exception if the libvirt call fails. So ACK to this bug fix we should put it in maint branches. Much as I hate to raise the issue this assumption is true for pointer APIs but APIs that return an integer don't raise an exception, they just return -1. Obviously, changing this behavior would be way too invasive but documenting this behavior should be done somewhere. What APIs in particular ? Any API which results in a libvirt error being set should be translated into an exception in the python. This is done whether they're APIs returning NULL pointers or -1 ints. If there are other exceptions to the rule they must be fixed too. 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] [python PATCH] maint: next release is 1.2.0
No other hits for: git grep '1\.1\.5' * libvirt-utils.h: Fix comment. Signed-off-by: Eric Blake ebl...@redhat.com --- Pushing under the trivial rule (and as a test that the new python repo is set up correctly). libvirt-utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-utils.h b/libvirt-utils.h index 0517f9c..f55be7b 100644 --- a/libvirt-utils.h +++ b/libvirt-utils.h @@ -29,7 +29,7 @@ # endif /** - * libvirt.h provides this as of version 1.1.5, but we want to be able + * libvirt.h provides this as of version 1.2.0, but we want to be able * to support older versions of libvirt so copy and paste the macro from * libvirt.h */ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 3/8] storage: implement rudimentary glusterfs pool refresh
On Fri, Nov 22, 2013 at 08:20:25PM -0700, Eric Blake wrote: Actually put gfapi to use, by allowing the creation of a gluster pool. Right now, all volumes are treated as raw and directories are skipped; further patches will allow peering into files to allow for qcow2 files and backing chains, and reporting proper volume allocation. This implementation was tested against Fedora 19's glusterfs 3.4.1; it might be made simpler by requiring a higher minimum, and/or require more hacks to work with a lower minimum. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshPool): Initial implementation. (virStorageBackendGlusterOpen, virStorageBackendGlusterClose) (virStorageBackendGlusterRefreshVol): New helper functions. Signed-off-by: Eric Blake ebl...@redhat.com 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
[libvirt] [PATCH] spec: Don't save/restore running VMs on libvirt-client update
The previous attempt (commit d65e0e1) removed just one of two
libvirt-guests restarts that happened on libvirt-client update. Let's
remove the last one too :-)
https://bugzilla.redhat.com/show_bug.cgi?id=962225
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index bbce8b5..d11b11f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1706,7 +1706,7 @@ fi
/sbin/ldconfig
%if %{with_systemd}
%if %{with_systemd_macros}
-%systemd_postun_with_restart libvirt-guests.service
+%systemd_postun libvirt-guests.service
%endif
%triggerun client -- libvirt 0.9.4
%{_bindir}/systemd-sysv-convert --save libvirt-guests /dev/null 21 ||:
--
1.8.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 4/8] storage: add network-dir as new storage volume type
On Mon, Nov 25, 2013 at 10:42:30AM +, Daniel P. Berrange wrote:
On Fri, Nov 22, 2013 at 08:20:26PM -0700, Eric Blake wrote:
In the 'directory' and 'netfs' storage pools, a user can see
both 'file' and 'dir' storage volume types, to know when they
can descend into a subdirectory. But in a network-based storage
pool, such as the upcoming 'gluster' pool, we use 'network'
instead of 'file', and did not have any counterpart for a
directory until this patch. Adding a new volume type
'network-dir' is better than reusing 'dir', because it makes
it clear that the only way to access 'network' volumes within
that container is through the network mounting (leaving 'dir'
for something accessible in the local file system).
* include/libvirt/libvirt.h.in (virStorageVolType): Expand enum.
* src/qemu/qemu_command.c (qemuBuildVolumeString): Fix client.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise.
* tools/virsh-volume.c (vshVolumeTypeToString): Likewise.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemVolDelete): Likewise.
Signed-off-by: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 2 ++
src/conf/storage_conf.c | 2 +-
src/qemu/qemu_command.c | 6 --
src/qemu/qemu_conf.c | 4 +++-
src/storage/storage_backend_fs.c | 5 +++--
tools/virsh-volume.c | 5 -
6 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 146a59b..5e8cba6 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2951,6 +2951,8 @@ typedef enum {
VIR_STORAGE_VOL_BLOCK = 1,/* Block based volumes */
VIR_STORAGE_VOL_DIR = 2, /* Directory-passthrough based volume */
VIR_STORAGE_VOL_NETWORK = 3, /* Network volumes like RBD (RADOS Block
Device) */
+VIR_STORAGE_VOL_NETWORK_DIR = 4, /* Network accessible directory that
can
+ * contain other network volumes */
#ifdef VIR_ENUM_SENTINELS
VIR_STORAGE_VOL_LAST
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index add2ae1..493a874 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -53,7 +53,7 @@
VIR_ENUM_IMPL(virStorageVol,
VIR_STORAGE_VOL_LAST,
- file, block, dir, network)
+ file, block, dir, network, network-dir)
I've got to say I really don't like this naming but not
got a better suggestion yet.
Could we at least shorten it to 'netdir' ?
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] Return right error code for baselineCPU
On Mon, Nov 25, 2013 at 03:48:45PM +, Daniel P. Berrange wrote: On Mon, Nov 25, 2013 at 08:40:41AM -0700, Don Dugger wrote: On Mon, Nov 25, 2013 at 10:45:38AM +, Daniel P. Berrange wrote: On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote: On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote: This Python interface code is returning a -1 on errors for the `baselineCPU' API. Since this API is supposed to return a pointer the error return value should really be VIR_PY_NONE. ... ACK. This is correct. But it obviously changes our API so I'm not really sure how we should handle this, (e.g. document the API as is as note that its broken or fix it). The implicit expectation with python APIs is that they all raise an exception if the libvirt call fails. So ACK to this bug fix we should put it in maint branches. Much as I hate to raise the issue this assumption is true for pointer APIs but APIs that return an integer don't raise an exception, they just return -1. Obviously, changing this behavior would be way too invasive but documenting this behavior should be done somewhere. What APIs in particular ? Any API which results in a libvirt error being set should be translated into an exception in the python. This is done whether they're APIs returning NULL pointers or -1 ints. If there are other exceptions to the rule they must be fixed too. I'm basing this on code inspection so I could be wrong but if you look at the Python interface code for libvirt_virDomainSetSchedulerParameters it checks the return value from virDomainSetSchedulerParameters and, if it is 0, then the Pythong code returns -1, without raising an exception. 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
Re: [libvirt] [PATCHv4 5/8] storage: improve directory support in gluster pool
On Fri, Nov 22, 2013 at 08:20:27PM -0700, Eric Blake wrote: diff --git a/tests/storagevolxml2xmlin/vol-gluster-dir.xml b/tests/storagevolxml2xmlin/vol-gluster-dir.xml new file mode 100644 index 000..bd20a6a --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-gluster-dir.xml @@ -0,0 +1,13 @@ +volume + namedir/name + key/vol/dir/key + source + /source + typenetwork-dir/type Per my other reply I think that 'type' would be better as an attribute on the top level volume element. 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] [PATCHv4 7/8] storage: improve handling of symlinks in gluster
On Fri, Nov 22, 2013 at 08:20:29PM -0700, Eric Blake wrote: With this patch, dangling and looping symlinks are silently ignored, while links to files and directories are treated the same as the underlying file or directory. This is the same behavior as both 'directory' and 'netfs' pools. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Treat symlinks similar to directory and netfs pools. Signed-off-by: Eric Blake ebl...@redhat.com --- src/storage/storage_backend_gluster.c | 11 +++ 1 file changed, 11 insertions(+) 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] [PATCHv4 6/8] storage: improve allocation stats reported on gluster files
On Fri, Nov 22, 2013 at 08:20:28PM -0700, Eric Blake wrote: We already had code for handling allocation different than capacity for sparse files; we just had to wire it up to be used when inspecting gluster images. * src/storage/storage_backend.c (virStorageBackendUpdateVolTargetInfoFD): Handle no fd. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Handle sparse files. Signed-off-by: Eric Blake ebl...@redhat.com --- src/storage/storage_backend.c | 8 src/storage/storage_backend_gluster.c | 7 ++- 2 files changed, 10 insertions(+), 5 deletions(-) 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] [PATCHv4 8/8] storage: probe qcow2 volumes in gluster pool
On Fri, Nov 22, 2013 at 08:20:30PM -0700, Eric Blake wrote:
Putting together pieces from previous patches, it is now possible
for 'virsh vol-dumpxml --pool gluster volname' to report metadata
about a qcow2 file stored on gluster. The backing file is still
treated as raw; to fix that, more patches are needed to make the
storage backing chain analysis recursive rather than halting at
a network protocol name, but that work will not need any further
calls into libgfapi so much as just reusing this code, and that
should be the only code outside of the storage driver that needs
any help from libgfapi. Any additional use of libgfapi within
libvirt should only be needed for implementing storage pool APIs
such as volume creation or resizing, where backing chain analysis
should be unaffected.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterReadHeader): New helper function.
(virStorageBackendGlusterRefreshVol): Probe non-raw files.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/storage/storage_backend_gluster.c | 76
++-
1 file changed, 75 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index e935583..b239880 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -147,6 +147,37 @@ error:
}
+static ssize_t
+virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
+ const char *name,
+ ssize_t maxlen,
+ char **buf)
+{
+char *s;
+size_t nread = 0;
+
+if (VIR_ALLOC_N(*buf, maxlen) 0)
+return -1;
+
+s = *buf;
+while (maxlen) {
+ssize_t r = glfs_read(fd, s, maxlen, 0);
+if (r 0 errno == EINTR)
+continue;
+if (r 0) {
+VIR_FREE(*buf);
+virReportSystemError(errno, _(unable to read '%s'), name);
+return r;
+}
Further down you're requesting O_NONBLOCK, and here you are
not handling EAGAIN explicitly. Is is desirable that we turn
EAGAIN into a fatal error, or should we remove the O_NONBLOCK
flag ?
@@ -208,15 +243,54 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
goto cleanup;
}
-/* FIXME - must open files to determine if they are non-raw */
vol-type = VIR_STORAGE_VOL_NETWORK;
vol-target.format = VIR_STORAGE_FILE_RAW;
+if (!(fd = glfs_open(state-vol, name, O_RDONLY| O_NONBLOCK |
O_NOCTTY))) {
+/* A dangling symlink now implies a TOCTTOU race; report it. */
+virReportSystemError(errno, _(cannot open volume '%s'), name);
+goto cleanup;
+}
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] Return right error code for baselineCPU
On 11/25/2013 08:55 AM, Don Dugger wrote:
I'm basing this on code inspection so I could be wrong but if you look
at the Python interface code for libvirt_virDomainSetSchedulerParameters
it checks the return value from virDomainSetSchedulerParameters and,
if it is 0, then the Pythong code returns -1, without raising an
exception.
We have two layers of python code. The libvirt-override.c layer returns
None/-1 if a libvirt error is present, then the generated layer turns
that into a python exception to the end user. Look at the generated
libvirt.py for a lot of examples of code that does:
if ret == -1: raise libvirtError ('... failed')
for anything where the C code fails with a -1, and does:
if ret is None: raise libvirtError ('... failed')
for anything where the C code fails with NULL.
The bug that we are fixing here is that we were returning -1 instead of
None for C code that fails with NULL, so the generated wrapper was not
properly throwing a libvirtError.
--
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] spec: Don't save/restore running VMs on libvirt-client update
On 11/25/2013 08:53 AM, Jiri Denemark wrote:
The previous attempt (commit d65e0e1) removed just one of two
libvirt-guests restarts that happened on libvirt-client update. Let's
remove the last one too :-)
https://bugzilla.redhat.com/show_bug.cgi?id=962225
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/libvirt.spec.in b/libvirt.spec.in
index bbce8b5..d11b11f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1706,7 +1706,7 @@ fi
/sbin/ldconfig
%if %{with_systemd}
%if %{with_systemd_macros}
-%systemd_postun_with_restart libvirt-guests.service
+%systemd_postun libvirt-guests.service
%endif
%triggerun client -- libvirt 0.9.4
%{_bindir}/systemd-sysv-convert --save libvirt-guests /dev/null 21 ||:
--
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] Return right error code for baselineCPU
On Mon, Nov 25, 2013 at 08:55:12AM -0700, Don Dugger wrote:
On Mon, Nov 25, 2013 at 03:48:45PM +, Daniel P. Berrange wrote:
On Mon, Nov 25, 2013 at 08:40:41AM -0700, Don Dugger wrote:
On Mon, Nov 25, 2013 at 10:45:38AM +, Daniel P. Berrange wrote:
On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote:
On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote:
This Python interface code is returning a -1 on errors for the
`baselineCPU' API. Since this API is supposed to return a pointer
the error return value should really be VIR_PY_NONE.
...
ACK. This is correct. But it obviously changes our API so I'm not
really sure how we should handle this, (e.g. document the API as is as
note that its broken or fix it).
The implicit expectation with python APIs is that they all raise an
exception if the libvirt call fails. So ACK to this bug fix we
should put it in maint branches.
Much as I hate to raise the issue this assumption is true for pointer
APIs but APIs that return an integer don't raise an exception, they
just return -1. Obviously, changing this behavior would be way too
invasive but documenting this behavior should be done somewhere.
What APIs in particular ? Any API which results in a libvirt error
being set should be translated into an exception in the python. This
is done whether they're APIs returning NULL pointers or -1 ints. If
there are other exceptions to the rule they must be fixed too.
I'm basing this on code inspection so I could be wrong but if you look
at the Python interface code for libvirt_virDomainSetSchedulerParameters
it checks the return value from virDomainSetSchedulerParameters and,
if it is 0, then the Pythong code returns -1, without raising an
exception.
That code is the low-level module (called libvirtmod) which interfaces
to the C library, and is not called directly by applications.
Applications call the main module (called libvirt) which translates
to the lowlevel module. eg in this case it does:
def setSchedulerParametersFlags(self, params, flags=0):
Change the scheduler parameters
ret = libvirtmod.virDomainSetSchedulerParametersFlags(self._o, params,
flags)
if ret == -1: raise libvirtError
('virDomainSetSchedulerParametersFlags() failed', dom=self)
return ret
so the -1 error is turned into an exception for applications.
The flaw the original patch in this thread was fixing is that the low
level libvirtmod was returning NULL, where the high level libvirt
expected to see -1, so the exception translation was missed.
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] spec: Don't save/restore running VMs on libvirt-client update
On Mon, Nov 25, 2013 at 09:00:43 -0700, Eric Blake wrote: On 11/25/2013 08:53 AM, Jiri Denemark wrote: The previous attempt (commit d65e0e1) removed just one of two libvirt-guests restarts that happened on libvirt-client update. Let's remove the last one too :-) https://bugzilla.redhat.com/show_bug.cgi?id=962225 Signed-off-by: Jiri Denemark jdene...@redhat.com --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: expose volume meta-type in XML
On 11/24/2013 09:57 PM, Osier Yang wrote: On 23/11/13 06:26, Eric Blake wrote: I got annoyed at having to use both 'virsh vol-list $pool --details' AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated the volume correctly. Since two-thirds of the data present in virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(), this just adds the remaining piece of information. * docs/formatstorage.html.in: Document new target type= I didn't see it relates with target. * docs/schemas/storagevol.rng (target, backingStore): Add it to RelaxNG. I thought (target, backingStore) means add type to both of them. Finally see it means between :-) Blah; stale commit comments. I'll fix them as part of addressing Dan's comment. + dtcodetype/code/dt + ddOutput-only; provides the volume type that is also available +from codevirStorageVolGetInfo()/code. span class=sinceSince I think it's better to mention virsh vol-list $pool --details instead of the API name here, as we did across the documents. I'm fine if you keep it though. The API name is more useful to anyone using bindings and not virsh. I'll keep it with the API name. +VIR_ENUM_IMPL(virStorageVol, + VIR_STORAGE_VOL_LAST, + file, block, dir, network) Here the network-dir type is not included though. So I guess you want to push this patch before the glusterfs series. ACK if the network-dir is removed. Indeed, I messed up in my rebasing. network-dir is not supposed to be present in this patch. v2 coming up. -- 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] storage: expose volume meta-type in XML
On 11/25/2013 03:41 AM, Daniel P. Berrange wrote: On Fri, Nov 22, 2013 at 03:26:11PM -0700, Eric Blake wrote: I got annoyed at having to use both 'virsh vol-list $pool --details' AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated the volume correctly. Since two-thirds of the data present in virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(), this just adds the remaining piece of information. * docs/formatstorage.html.in: Document new target type= * docs/schemas/storagevol.rng (target, backingStore): Add it to RelaxNG. * src/conf/storage_conf.h (virStorageVolTypeToString): Declare. * src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output the metatype. * tests/storagevolxml2xmlout/vol-*.xml: Update tests to match. Signed-off-by: Eric Blake ebl...@redhat.com --- I think I'd be more inclined to have a top level attribute eg volume type=block I debated about that, but your answer swayed me. Consider it done, v2 coming up. -- 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
[libvirt] [PATCH 00/22] Misc refactors and cleanups leading to gluster snapshot support
Peter Krempa (22): conf: Implement virStorageVolType enum helper functions test: Implement fake storage pool driver in qemuxml2argv test qemuxml2argv: Add test to verify correct usage of disk type=volume qemuxml2argv: Add test for disk type='volume' with iSCSI pools qemu: Refactor qemuTranslatePool source qemu: Split out formatting of network disk source URI qemu: Simplify call pattern of qemuBuildDriveURIString qemu: Use qemuBuildNetworkDriveURI to handle http/ftp and friends qemu: Migrate sheepdog source generation into common function qemu: Split out NBD command generation qemu: Unify formatting of RBD sources qemu: Refactor disk source string formatting conf: Support disk source formatting without needing a virDomainDiskDefPtr conf: Clean up virDomainDiskSourceDefFormatInternal conf: Split out seclabel formating code for disk source conf: Export disk source formatter and parser snapshot: conf: Use common parsing and formatting functions for source snapshot: conf: Fix NULL dereference when driver element is empty conf: Add functions to copy and free network disk source definitions qemu: snapshot: Detect internal snapshots also for sheepdog and RBD conf: Add helper do clear disk source authentication struct qemu: Clear old translated pool source src/conf/domain_conf.c | 261 ++--- src/conf/domain_conf.h | 25 + src/conf/snapshot_conf.c | 56 +- src/conf/snapshot_conf.h | 1 + src/conf/storage_conf.c| 4 + src/conf/storage_conf.h| 2 + src/libvirt_private.syms | 6 + src/qemu/qemu_command.c| 650 +++-- src/qemu/qemu_command.h| 6 + src/qemu/qemu_conf.c | 129 ++-- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_driver.c | 3 +- .../qemuxml2argv-disk-source-pool-mode.args| 10 + .../qemuxml2argv-disk-source-pool-mode.xml | 4 +- .../qemuxml2argv-disk-source-pool.args | 8 + .../qemuxml2argv-disk-source-pool.xml | 2 +- tests/qemuxml2argvtest.c | 166 ++ 17 files changed, 874 insertions(+), 461 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args -- 1.8.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/22] conf: Support disk source formatting without needing a virDomainDiskDefPtr
The source element formatting function was expecting a
virDomainDiskDefPtr to store the data. As snapshots are not using this
data structure to hold the data, we need to add an internal function
which splits out individual fields separately.
---
src/conf/domain_conf.c | 131 +
1 file changed, 79 insertions(+), 52 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7b0e3ea..a6d7600 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14352,28 +14352,38 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
}
static int
-virDomainDiskSourceDefFormat(virBufferPtr buf,
- virDomainDiskDefPtr def,
- unsigned int flags)
+virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
+ int type,
+ const char *src,
+ int policy,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ size_t nseclabels,
+ virSecurityDeviceLabelDefPtr *seclabels,
+ virDomainDiskSourcePoolDefPtr srcpool,
+ unsigned int flags)
{
-int n;
-const char *startupPolicy =
virDomainStartupPolicyTypeToString(def-startupPolicy);
+size_t n;
+const char *startupPolicy = NULL;
-if (def-src || def-nhosts 0 || def-srcpool ||
-def-startupPolicy) {
-switch (def-type) {
+if (policy)
+startupPolicy = virDomainStartupPolicyTypeToString(policy);
+
+if (src || nhosts 0 || srcpool || startupPolicy) {
+switch (type) {
case VIR_DOMAIN_DISK_TYPE_FILE:
-virBufferAddLit(buf, source);
-if (def-src)
-virBufferEscapeString(buf, file='%s', def-src);
-if (def-startupPolicy)
+virBufferAddLit(buf, source);
+if (src)
+virBufferEscapeString(buf, file='%s', src);
+if (startupPolicy)
virBufferEscapeString(buf, startupPolicy='%s',
startupPolicy);
-if (def-nseclabels) {
+if (nseclabels) {
virBufferAddLit(buf, \n);
virBufferAdjustIndent(buf, 8);
-for (n = 0; n def-nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, def-seclabels[n],
+for (n = 0; n nseclabels; n++)
+virSecurityDeviceLabelDefFormat(buf, seclabels[n],
flags);
virBufferAdjustIndent(buf, -8);
virBufferAddLit(buf, /source\n);
@@ -14383,15 +14393,15 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
break;
case VIR_DOMAIN_DISK_TYPE_BLOCK:
virBufferAddLit(buf, source);
-virBufferEscapeString(buf, dev='%s', def-src);
-if (def-startupPolicy)
+virBufferEscapeString(buf, dev='%s', src);
+if (startupPolicy)
virBufferEscapeString(buf, startupPolicy='%s',
startupPolicy);
-if (def-nseclabels) {
+if (nseclabels) {
virBufferAddLit(buf, \n);
virBufferAdjustIndent(buf, 8);
-for (n = 0; n def-nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, def-seclabels[n],
+for (n = 0; n nseclabels; n++)
+virSecurityDeviceLabelDefFormat(buf, seclabels[n],
flags);
virBufferAdjustIndent(buf, -8);
virBufferAddLit(buf, /source\n);
@@ -14400,41 +14410,38 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
}
break;
case VIR_DOMAIN_DISK_TYPE_DIR:
-virBufferEscapeString(buf, source dir='%s',
- def-src);
-if (def-startupPolicy)
+virBufferEscapeString(buf, source dir='%s', src);
+if (startupPolicy)
virBufferEscapeString(buf, startupPolicy='%s',
startupPolicy);
virBufferAddLit(buf, /\n);
break;
case VIR_DOMAIN_DISK_TYPE_NETWORK:
virBufferAsprintf(buf, source protocol='%s',
-
virDomainDiskProtocolTypeToString(def-protocol));
-if (def-src) {
-virBufferEscapeString(buf, name='%s', def-src);
-}
-if (def-nhosts == 0) {
+ virDomainDiskProtocolTypeToString(protocol));
+
[libvirt] [PATCH 18/22] snapshot: conf: Fix NULL dereference when driver element is empty
Consider the following valid snapshot XML as the driver element is
allowed to be empty in the domainsnapshot.rng schema:
$ cat snap.xml
domainsnapshot
disks
disk name='vda' snapshot='external'
source file='/tmp/foo'/
driver/
/disk
/disks
/domainsnapshot
produces the following error:
$ virsh snapshot-create domain snap.xml
error: internal error: unknown disk snapshot driver '(null)'
The driver type is parsed as NULL from the XML as the attribute is not
present and then directly used to produce the error message.
With this patch the attempt to parse the driver type is skipped if not
present to avoid changing the schema to forbid the empty driver element.
---
src/conf/snapshot_conf.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 7258daa..5958f13 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -154,15 +154,17 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
} else if (!def-format
xmlStrEqual(cur-name, BAD_CAST driver)) {
char *driver = virXMLPropString(cur, type);
-def-format = virStorageFileFormatTypeFromString(driver);
-if (def-format = 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(unknown disk snapshot driver '%s'),
- driver);
+if (driver) {
+def-format = virStorageFileFormatTypeFromString(driver);
+if (def-format = 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unknown disk snapshot driver '%s'),
+ driver);
+VIR_FREE(driver);
+goto cleanup;
+}
VIR_FREE(driver);
-goto cleanup;
}
-VIR_FREE(driver);
}
}
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/22] qemu: Split out NBD command generation
---
src/qemu/qemu_command.c | 117 +---
1 file changed, 61 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 82fce0e..afb8fe6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3680,7 +3680,7 @@ qemuNetworkDriveGetPort(int protocol,
return -1;
}
-
+#define QEMU_DEFAULT_NBD_PORT 10809
char *
qemuBuildNetworkDriveURI(int protocol,
@@ -3691,9 +3691,67 @@ qemuBuildNetworkDriveURI(int protocol,
const char *secret)
{
char *ret = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
virURIPtr uri = NULL;
switch ((enum virDomainDiskProtocol) protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+if (nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(protocol '%s' accepts only one host),
+ virDomainDiskProtocolTypeToString(protocol));
+goto cleanup;
+}
+
+if (!((hosts-name strchr(hosts-name, ':')) ||
+ (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP
+ !hosts-name) ||
+ (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX
+ hosts-socket
+ hosts-socket[0] != '/'))) {
+
+virBufferAddLit(buf, nbd:);
+
+switch (hosts-transport) {
+case VIR_DOMAIN_DISK_PROTO_TRANS_TCP:
+virBufferStrcat(buf, hosts-name, NULL);
+virBufferAsprintf(buf, :%s,
+ hosts-port ? hosts-port :
+ QEMU_DEFAULT_NBD_PORT);
+break;
+
+case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
+if (!hosts-socket) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(socket attribute required for
+ nix transport));
+goto cleanup;
+}
+
+virBufferAsprintf(buf, unix:%s, hosts-socket);
+break;
+
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(nbd does not support transport '%s'),
+
virDomainDiskProtocolTransportTypeToString(hosts-transport));
+goto cleanup;
+}
+
+if (src)
+virBufferAsprintf(buf, :exportname=%s, src);
+
+if (virBufferError(buf) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+ret = virBufferContentAndReset(buf);
+goto cleanup;
+}
+/* fallthrough */
+/* NBD code uses same formatting scheme as others in some cases */
+
case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
case VIR_DOMAIN_DISK_PROTOCOL_FTP:
@@ -3701,7 +3759,6 @@ qemuBuildNetworkDriveURI(int protocol,
case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
-case VIR_DOMAIN_DISK_PROTOCOL_NBD:
if (nhosts != 1) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(protocol '%s' accepts only one host),
@@ -3786,6 +3843,7 @@ qemuBuildNetworkDriveURI(int protocol,
}
cleanup:
+virBufferFreeAndReset(buf);
virURIFree(uri);
return ret;
@@ -3839,56 +3897,6 @@ cleanup:
}
-#define QEMU_DEFAULT_NBD_PORT 10809
-
-static int
-qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr
opt)
-{
-const char *transp;
-
-if (disk-nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(nbd accepts only one host));
-return -1;
-}
-
-if ((disk-hosts-name strchr(disk-hosts-name, ':')) ||
-(disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP
!disk-hosts-name) ||
-(disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX
disk-hosts-socket disk-hosts-socket[0] != '/'))
-return qemuBuildDriveURIString(conn, disk, opt);
-
-virBufferAddLit(opt, file=nbd:);
-
-switch (disk-hosts-transport) {
-case VIR_DOMAIN_DISK_PROTO_TRANS_TCP:
-if (disk-hosts-name)
-virBufferEscape(opt, ',', ,, %s, disk-hosts-name);
-virBufferEscape(opt, ',', ,, :%s,
-disk-hosts-port ? disk-hosts-port :
-QEMU_DEFAULT_NBD_PORT);
-break;
-case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
-if (!disk-hosts-socket) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(socket attribute required for unix transport));
[libvirt] [PATCH 06/22] qemu: Split out formatting of network disk source URI
The snapshot code will need to use qemu-style formatted URIs of network
disks. Split out the code to avoid duplication.
---
src/qemu/qemu_command.c | 146
src/qemu/qemu_command.h | 6 ++
2 files changed, 91 insertions(+), 61 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2a4390e..91b1bd2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3626,106 +3626,125 @@ error:
return -1;
}
-static int
-qemuBuildDriveURIString(virConnectPtr conn,
-virDomainDiskDefPtr disk, virBufferPtr opt,
-const char *scheme, virSecretUsageType secretUsageType)
-{
-int ret = -1;
-int port = 0;
-char *secret = NULL;
-char *tmpscheme = NULL;
-char *volimg = NULL;
-char *sock = NULL;
-char *user = NULL;
-char *builturi = NULL;
-const char *transp = NULL;
-virURI uri = {
-.port = port /* just to clear rest of bits */
-};
+char *
+qemuBuildNetworkDriveURI(int protocol,
+ const char *src,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ const char *username,
+ const char *secret)
+{
+char *ret = NULL;
+virURIPtr uri = NULL;
-if (disk-nhosts != 1) {
+if (nhosts != 1) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(%s accepts only one host), scheme);
-return -1;
+ _(protocol '%s' accepts only one host),
+ virDomainDiskProtocolTypeToString(protocol));
+return NULL;
}
-virBufferAddLit(opt, file=);
-transp =
virDomainDiskProtocolTransportTypeToString(disk-hosts-transport);
+if (VIR_ALLOC(uri) 0)
+return NULL;
-if (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
-if (VIR_STRDUP(tmpscheme, scheme) 0)
+if (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
+if (VIR_STRDUP(uri-scheme,
virDomainDiskProtocolTypeToString(protocol)) 0)
goto cleanup;
} else {
-if (virAsprintf(tmpscheme, %s+%s, scheme, transp) 0)
+if (virAsprintf(uri-scheme, %s+%s,
+virDomainDiskProtocolTypeToString(protocol),
+
virDomainDiskProtocolTransportTypeToString(hosts-transport)) 0)
goto cleanup;
}
-if (disk-src virAsprintf(volimg, /%s, disk-src) 0)
+if (src
+virAsprintf(uri-path, /%s, src) 0)
+goto cleanup;
+
+if (hosts-port
+virStrToLong_i(hosts-port, NULL, 10, uri-port) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(failed to parse port number '%s'),
+ hosts-port);
+goto cleanup;
+}
+
+if (hosts-socket
+virAsprintf(uri-query, socket=%s, hosts-socket) 0)
goto cleanup;
-if (disk-hosts-port) {
-port = atoi(disk-hosts-port);
+if (username) {
+if (secret) {
+if (virAsprintf(uri-user, %s:%s, username, secret) 0)
+goto cleanup;
+} else {
+if (VIR_STRDUP(uri-user, username) 0)
+goto cleanup;
+}
}
-if (disk-hosts-socket
-virAsprintf(sock, socket=%s, disk-hosts-socket) 0)
+if (VIR_STRDUP(uri-server, hosts-name) 0)
goto cleanup;
+ret = virURIFormat(uri);
+
+cleanup:
+virURIFree(uri);
+
+return ret;
+}
+
+
+static int
+qemuBuildDriveURIString(virConnectPtr conn,
+virDomainDiskDefPtr disk,
+virBufferPtr opt,
+int protocol,
+virSecretUsageType secretUsageType)
+{
+char *secret = NULL;
+char *builturi = NULL;
+int ret = -1;
+
+virBufferAddLit(opt, file=);
+
if (disk-auth.username secretUsageType != VIR_SECRET_USAGE_TYPE_NONE) {
/* Get the secret string using the virDomainDiskDef */
-if (!(secret = qemuGetSecretString(conn, scheme, false,
+if (!(secret = qemuGetSecretString(conn,
+
virDomainDiskProtocolTypeToString(protocol),
+ false,
disk-auth.secretType,
disk-auth.username,
disk-auth.secret.uuid,
disk-auth.secret.usage,
secretUsageType)))
goto cleanup;
-if (virAsprintf(user, %s:%s, disk-auth.username, secret) 0)
-goto cleanup;
}
-uri.scheme = tmpscheme; /* gluster+transport */
-uri.server = disk-hosts-name;
-uri.user = user;
-uri.port = port;
-uri.path = volimg;
-uri.query = sock;
+if (!(builturi =
[libvirt] [PATCH 04/22] qemuxml2argv: Add test for disk type='volume' with iSCSI pools
Tweak the existing file so that it can be tested for command line corectness. --- tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args | 10 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml | 4 ++-- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args new file mode 100644 index 000..8f6a3dd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/some/block/device/unit:0:0:1,if=none,media=cdrom,id=drive-ide0-0-1 -device \ +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \ +file=iscsi://iscsi.example.com:3260/demo-target/2,if=none,media=cdrom,id=drive-ide0-0-2 \ +-device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -drive \ +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-3 -device \ +ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml index b907633..9f90293 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml @@ -15,7 +15,7 @@ devices emulator/usr/bin/qemu/emulator disk type='volume' device='cdrom' - source pool='blk-pool0' volume='blk-pool0-vol0' mode='host' startupPolicy='optional' + source pool='pool-iscsi-auth' volume='unit:0:0:1' mode='host' seclabel model='selinux' relabel='yes' labelsystem_u:system_r:public_content_t:s0/label /seclabel @@ -25,7 +25,7 @@ address type='drive' controller='0' bus='0' target='0' unit='1'/ /disk disk type='volume' device='cdrom' - source pool='blk-pool0' volume='blk-pool0-vol1' mode='direct' startupPolicy='optional' + source pool='pool-iscsi' volume='unit:0:0:2' mode='direct' seclabel model='selinux' relabel='yes' labelsystem_u:system_r:public_content_t:s0/label /seclabel diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b90babf..2fcd707 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -772,6 +772,8 @@ mymain(void) QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT); DO_TEST(disk-source-pool, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +DO_TEST(disk-source-pool-mode, +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-ioeventfd, QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_IOEVENTFD, QEMU_CAPS_VIRTIO_TX_ALG, QEMU_CAPS_DEVICE, -- 1.8.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/22] conf: Clean up virDomainDiskSourceDefFormatInternal
Avoid if statements when used with virBufferEscapeString which
automaticaly omits the whole string. Also add some line breaks to
visualy separate the code.
---
src/conf/domain_conf.c | 50 +-
1 file changed, 21 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a6d7600..03663e4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14373,54 +14373,50 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
if (src || nhosts 0 || srcpool || startupPolicy) {
switch (type) {
case VIR_DOMAIN_DISK_TYPE_FILE:
-virBufferAddLit(buf, source);
-if (src)
-virBufferEscapeString(buf, file='%s', src);
-if (startupPolicy)
-virBufferEscapeString(buf, startupPolicy='%s',
- startupPolicy);
+virBufferAddLit(buf, source);
+virBufferEscapeString(buf, file='%s', src);
+virBufferEscapeString(buf, startupPolicy='%s', startupPolicy);
+
if (nseclabels) {
virBufferAddLit(buf, \n);
virBufferAdjustIndent(buf, 8);
for (n = 0; n nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n],
-flags);
+virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
virBufferAdjustIndent(buf, -8);
virBufferAddLit(buf, /source\n);
} else {
virBufferAddLit(buf, /\n);
}
break;
+
case VIR_DOMAIN_DISK_TYPE_BLOCK:
virBufferAddLit(buf, source);
virBufferEscapeString(buf, dev='%s', src);
-if (startupPolicy)
-virBufferEscapeString(buf, startupPolicy='%s',
- startupPolicy);
+virBufferEscapeString(buf, startupPolicy='%s', startupPolicy);
+
if (nseclabels) {
virBufferAddLit(buf, \n);
virBufferAdjustIndent(buf, 8);
for (n = 0; n nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n],
-flags);
+virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
virBufferAdjustIndent(buf, -8);
virBufferAddLit(buf, /source\n);
} else {
virBufferAddLit(buf, /\n);
}
break;
+
case VIR_DOMAIN_DISK_TYPE_DIR:
-virBufferEscapeString(buf, source dir='%s', src);
-if (startupPolicy)
-virBufferEscapeString(buf, startupPolicy='%s',
- startupPolicy);
+virBufferAddLit(buf, source);
+virBufferEscapeString(buf, dir='%s', src);
+virBufferEscapeString(buf, startupPolicy='%s', startupPolicy);
virBufferAddLit(buf, /\n);
break;
+
case VIR_DOMAIN_DISK_TYPE_NETWORK:
virBufferAsprintf(buf, source protocol='%s',
virDomainDiskProtocolTypeToString(protocol));
-if (src)
-virBufferEscapeString(buf, name='%s', src);
+virBufferEscapeString(buf, name='%s', src);
if (nhosts == 0) {
virBufferAddLit(buf, /\n);
@@ -14428,25 +14424,21 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
virBufferAddLit(buf, \n);
for (n = 0; n nhosts; n++) {
virBufferAddLit(buf, host);
-if (hosts[n].name)
-virBufferEscapeString(buf, name='%s',
hosts[n].name);
-
-if (hosts[n].port)
-virBufferEscapeString(buf, port='%s',
- hosts[n].port);
+virBufferEscapeString(buf, name='%s', hosts[n].name);
+virBufferEscapeString(buf, port='%s', hosts[n].port);
if (hosts[n].transport)
virBufferAsprintf(buf, transport='%s',
virDomainDiskProtocolTransportTypeToString(hosts[n].transport));
-if (hosts[n].socket)
-virBufferEscapeString(buf, socket='%s',
hosts[n].socket);
+virBufferEscapeString(buf, socket='%s',
hosts[n].socket);
virBufferAddLit(buf, /\n);
}
virBufferAddLit(buf, /source\n);
}
break;
+
case VIR_DOMAIN_DISK_TYPE_VOLUME:
virBufferAddLit(buf, source);
@@ -14457,8 +14449,7 @@
[libvirt] [PATCH 12/22] qemu: Refactor disk source string formatting
---
src/qemu/qemu_command.c | 128
1 file changed, 86 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9704c90..7a6e322 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3827,19 +3827,62 @@ cleanup:
static int
-qemuBuildDriveURIString(virConnectPtr conn,
-virDomainDiskDefPtr disk,
-virBufferPtr opt)
+qemuGetDriveSourceString(int type,
+ const char *src,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ const char *username,
+ const char *secret,
+ char **path)
{
+*path = NULL;
+
+switch ((enum virDomainDiskType) type) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+case VIR_DOMAIN_DISK_TYPE_DIR:
+if (!src)
+return 1;
+
+if (VIR_STRDUP(*path, src) 0)
+return -1;
+
+break;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+if (!(*path = qemuBuildNetworkDriveURI(protocol,
+ src,
+ nhosts,
+ hosts,
+ username,
+ secret)))
+return -1;
+break;
+
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+break;
+}
+
+return 0;
+}
+
+static int
+qemuDomainDiskGetSourceString(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ char **source)
+{
+int actualType = qemuDiskGetActualType(disk);
char *secret = NULL;
-char *builturi = NULL;
int ret = -1;
-virBufferAddLit(opt, file=);
+*source = NULL;
-if ((disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI ||
- disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)
-disk-auth.username) {
+if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK
+disk-auth.username
+(disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI ||
+ disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) {
bool encode = false;
int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
@@ -3860,32 +3903,23 @@ qemuBuildDriveURIString(virConnectPtr conn,
goto cleanup;
}
-
-if (!(builturi = qemuBuildNetworkDriveURI(disk-protocol,
- disk-src,
- disk-nhosts,
- disk-hosts,
- disk-auth.username,
- secret)))
-goto cleanup;
-
-virBufferEscape(opt, ',', ,, %s, builturi);
-virBufferAddChar(opt, ',');
-
-ret = 0;
+ret = qemuGetDriveSourceString(qemuDiskGetActualType(disk),
+ disk-src,
+ disk-protocol,
+ disk-nhosts,
+ disk-hosts,
+ disk-auth.username,
+ secret,
+ source);
cleanup:
VIR_FREE(secret);
-VIR_FREE(builturi);
-
return ret;
}
-
-
char *
-qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
+qemuBuildDriveStr(virConnectPtr conn,
virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps)
@@ -3896,6 +3930,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainDiskGeometryTransTypeToString(disk-geometry.trans);
int idx = virDiskNameToIndex(disk-dst);
int busid = -1, unitid = -1;
+char *source = NULL;
int actualType = qemuDiskGetActualType(disk);
if (idx 0) {
@@ -3978,15 +4013,18 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
break;
}
-/* disk-src is NULL when we use nbd disks */
-if ((disk-src ||
-(actualType == VIR_DOMAIN_DISK_TYPE_NETWORK
- disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD))
+if (qemuDomainDiskGetSourceString(conn, disk, source) 0)
+goto error;
+
+if (source
!((disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM)
disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
-if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) {
+virBufferAddLit(opt, file=);
+
+switch (actualType) {
+case VIR_DOMAIN_DISK_TYPE_DIR:
/* QEMU only supports magic FAT format for now */
if (disk-format 0 disk-format != VIR_STORAGE_FILE_FAT) {
[libvirt] [PATCH 16/22] conf: Export disk source formatter and parser
This code will be reused in the snapshot disk definition parser. --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 20 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d000f97..09ea51f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4737,7 +4737,7 @@ cleanup: } -static int +int virDomainDiskSourceDefParse(xmlNodePtr node, int type, char **source, @@ -14377,7 +14377,7 @@ virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf, } } -static int +int virDomainDiskSourceDefFormatInternal(virBufferPtr buf, int type, const char *src, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a5ef2ca..e9800a5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2345,6 +2345,18 @@ int virDomainDefFormatInternal(virDomainDefPtr def, unsigned int flags, virBufferPtr buf); +int virDomainDiskSourceDefFormatInternal(virBufferPtr buf, + int type, + const char *src, + int policy, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + size_t nseclabels, + virSecurityDeviceLabelDefPtr *seclabels, + virDomainDiskSourcePoolDefPtr srcpool, + unsigned int flags); + int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev); @@ -2379,6 +2391,14 @@ virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); virDomainDiskDefPtr virDomainDiskRemoveByName(virDomainDefPtr def, const char *name); +int virDomainDiskSourceDefParse(xmlNodePtr node, +int type, +char **source, +int *proto, +size_t *nhosts, +virDomainDiskHostDefPtr *hosts, +virDomainDiskSourcePoolDefPtr *srcpool); + bool virDomainHasDiskMirror(virDomainObjPtr vm); int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net); -- 1.8.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/22] qemu: Migrate sheepdog source generation into common function
---
src/qemu/qemu_command.c | 37 -
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b1c7803..82fce0e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3754,6 +3754,29 @@ qemuBuildNetworkDriveURI(int protocol,
break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+if (!src) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(missing disk source for 'sheepdog'
protocol));
+goto cleanup;
+}
+
+if (nhosts == 0) {
+if (virAsprintf(ret, sheepdog:%s, src) 0)
+goto cleanup;
+} else if (nhosts == 1) {
+if (virAsprintf(ret, sheepdog:%s:%s:%s,
+hosts-name,
+hosts-port ? hosts-port : 7000,
+src) 0)
+goto cleanup;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(protocol 'sheepdog' accepts up to one
host));
+goto cleanup;
+}
+
+break;
+
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
case VIR_DOMAIN_DISK_PROTOCOL_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4001,6 +4024,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
virBufferAddChar(opt, ',');
break;
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
case VIR_DOMAIN_DISK_PROTOCOL_FTP:
@@ -4011,19 +4035,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
if (qemuBuildDriveURIString(conn, disk, opt) 0)
goto error;
break;
-
-case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
-if (disk-nhosts == 0) {
-virBufferEscape(opt, ',', ,, file=sheepdog:%s,,
-disk-src);
-} else {
-/* only one host is supported now */
-virBufferAsprintf(opt, file=sheepdog:%s:%s:,
- disk-hosts-name,
- disk-hosts-port ? disk-hosts-port :
7000);
-virBufferEscape(opt, ',', ,, %s,, disk-src);
-}
-break;
}
} else {
if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK)
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/22] conf: Implement virStorageVolType enum helper functions
Add support for string conversion from and to the virStorageVolType
enum.
---
src/conf/storage_conf.c | 4
src/conf/storage_conf.h | 2 ++
src/libvirt_private.syms | 2 ++
3 files changed, 8 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8b378c2..1355056 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -99,6 +99,10 @@ VIR_ENUM_IMPL(virStoragePoolAuthType,
VIR_STORAGE_POOL_AUTH_LAST,
none, chap, ceph)
+VIR_ENUM_IMPL(virStorageVol,
+ VIR_STORAGE_VOL_LAST,
+ file, block, dir, network);
+
typedef const char *(*virStorageVolFormatToString)(int format);
typedef int (*virStorageVolFormatFromString)(const char *format);
typedef const char *(*virStorageVolFeatureToString)(int feature);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index f062bd8..9897c97 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -550,6 +550,8 @@ enum virStoragePartedFsType {
};
VIR_ENUM_DECL(virStoragePartedFsType)
+VIR_ENUM_DECL(virStorageVol);
+
# define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE \
(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | \
VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a705c56..205fe56 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -701,6 +701,8 @@ virStorageVolDefFree;
virStorageVolDefParseFile;
virStorageVolDefParseNode;
virStorageVolDefParseString;
+virStorageVolTypeFromString;
+virStorageVolTypeToString;
# conf/storage_encryption_conf.h
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/22] qemuxml2argv: Add test to verify correct usage of disk type=volume
Tweak the existing file to test command line generator too. --- tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args | 8 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml | 2 +- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args new file mode 100644 index 000..da87ad9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/some/block/device/cdrom,if=none,media=cdrom,id=drive-ide0-0-1 -device \ +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \ +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-2 -device \ +ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml index 465a539..6ca5cf7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml @@ -15,7 +15,7 @@ devices emulator/usr/bin/qemu/emulator disk type='volume' device='cdrom' - source pool='blk-pool0' volume='blk-pool0-vol0' startupPolicy='optional' + source pool='pool-disk' volume='block+cdrom' seclabel model='selinux' relabel='yes' labelsystem_u:system_r:public_content_t:s0/label /seclabel diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 07640db..b90babf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -770,6 +770,8 @@ mymain(void) DO_TEST(disk-aio, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_AIO, QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT); +DO_TEST(disk-source-pool, +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-ioeventfd, QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_IOEVENTFD, QEMU_CAPS_VIRTIO_TX_ALG, QEMU_CAPS_DEVICE, -- 1.8.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/22] qemu: Unify formatting of RBD sources
---
src/qemu/qemu_command.c | 155 +++-
1 file changed, 61 insertions(+), 94 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index afb8fe6..9704c90 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3253,72 +3253,6 @@ cleanup:
return secret;
}
-static int
-qemuBuildRBDString(virConnectPtr conn,
- virDomainDiskDefPtr disk,
- virBufferPtr opt)
-{
-size_t i;
-int ret = 0;
-char *secret = NULL;
-
-if (strchr(disk-src, ':')) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(':' not allowed in RBD source volume name '%s'),
- disk-src);
-return -1;
-}
-
-virBufferEscape(opt, ',', ,, rbd:%s, disk-src);
-if (disk-auth.username) {
-virBufferEscape(opt, '\\', :, :id=%s, disk-auth.username);
-/* Get the secret string using the virDomainDiskDef
- * NOTE: qemu/librbd wants it base64 encoded
- */
-if (!(secret = qemuGetSecretString(conn, rbd, true,
- disk-auth.secretType,
- disk-auth.username,
- disk-auth.secret.uuid,
- disk-auth.secret.usage,
- VIR_SECRET_USAGE_TYPE_CEPH)))
-goto error;
-
-
-virBufferEscape(opt, '\\', :,
-:key=%s:auth_supported=cephx\\;none,
-secret);
-} else {
-virBufferAddLit(opt, :auth_supported=none);
-}
-
-if (disk-nhosts 0) {
-virBufferAddLit(opt, :mon_host=);
-for (i = 0; i disk-nhosts; ++i) {
-if (i) {
-virBufferAddLit(opt, \\;);
-}
-
-/* assume host containing : is ipv6 */
-if (strchr(disk-hosts[i].name, ':')) {
-virBufferEscape(opt, '\\', :, [%s], disk-hosts[i].name);
-} else {
-virBufferAsprintf(opt, %s, disk-hosts[i].name);
-}
-if (disk-hosts[i].port) {
-virBufferAsprintf(opt, \\:%s, disk-hosts[i].port);
-}
-}
-}
-
-cleanup:
-VIR_FREE(secret);
-
-return ret;
-
-error:
-ret = -1;
-goto cleanup;
-}
static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport)
{
@@ -3693,6 +3627,7 @@ qemuBuildNetworkDriveURI(int protocol,
char *ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
virURIPtr uri = NULL;
+size_t i;
switch ((enum virDomainDiskProtocol) protocol) {
case VIR_DOMAIN_DISK_PROTOCOL_NBD:
@@ -3835,10 +3770,51 @@ qemuBuildNetworkDriveURI(int protocol,
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+if (strchr(src, ':')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(':' not allowed in RBD source volume name
'%s'),
+ src);
+goto cleanup;
+}
+
+virBufferStrcat(buf, rbd:, src, NULL);
+
+if (username) {
+virBufferEscape(buf, '\\', :, :id=%s, username);
+virBufferEscape(buf, '\\', :,
+:key=%s:auth_supported=cephx\\;none,
+secret);
+} else {
+virBufferAddLit(buf, :auth_supported=none);
+}
+
+if (nhosts 0) {
+virBufferAddLit(buf, :mon_host=);
+for (i = 0; i nhosts; i++) {
+if (i)
+virBufferAddLit(buf, \\;);
+
+/* assume host containing : is ipv6 */
+if (strchr(hosts[i].name, ':'))
+virBufferEscape(buf, '\\', :, [%s],
hosts[i].name);
+else
+virBufferAsprintf(buf, %s, hosts[i].name);
+
+if (hosts[i].port)
+virBufferAsprintf(buf, \\:%s, hosts[i].port);
+}
+}
+
+if (virBufferError(buf) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+ret = virBufferContentAndReset(buf);
+break;
+
+
case VIR_DOMAIN_DISK_PROTOCOL_LAST:
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(network disk protocol '%s' not supported),
- virDomainDiskProtocolTypeToString(protocol));
goto cleanup;
}
@@ -3861,17 +3837,26 @@ qemuBuildDriveURIString(virConnectPtr conn,
virBufferAddLit(opt, file=);
-if (disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI
+if ((disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI ||
+ disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)
disk-auth.username) {
-
[libvirt] [PATCH 15/22] conf: Split out seclabel formating code for disk source
The code is common for all the various disk types. Split it out to a
common function.
---
src/conf/domain_conf.c | 62 --
1 file changed, 30 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 03663e4..d000f97 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14351,6 +14351,32 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
}
}
+
+/* virDomainDiskSourceDefFormatSeclabel:
+ *
+ * This function automaticaly closes the source element and formats any
+ * possible seclabels.
+ */
+static void
+virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf,
+ size_t nseclabels,
+ virSecurityDeviceLabelDefPtr *seclabels,
+ unsigned int flags)
+{
+size_t n;
+
+if (nseclabels) {
+virBufferAddLit(buf, \n);
+virBufferAdjustIndent(buf, 8);
+for (n = 0; n nseclabels; n++)
+virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
+virBufferAdjustIndent(buf, -8);
+virBufferAddLit(buf, /source\n);
+} else {
+virBufferAddLit(buf, /\n);
+}
+}
+
static int
virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
int type,
@@ -14377,33 +14403,15 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
virBufferEscapeString(buf, file='%s', src);
virBufferEscapeString(buf, startupPolicy='%s', startupPolicy);
-if (nseclabels) {
-virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 8);
-for (n = 0; n nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
-virBufferAdjustIndent(buf, -8);
-virBufferAddLit(buf, /source\n);
-} else {
-virBufferAddLit(buf, /\n);
-}
-break;
+virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels,
flags);
+ break;
case VIR_DOMAIN_DISK_TYPE_BLOCK:
virBufferAddLit(buf, source);
virBufferEscapeString(buf, dev='%s', src);
virBufferEscapeString(buf, startupPolicy='%s', startupPolicy);
-if (nseclabels) {
-virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 8);
-for (n = 0; n nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
-virBufferAdjustIndent(buf, -8);
-virBufferAddLit(buf, /source\n);
-} else {
-virBufferAddLit(buf, /\n);
-}
+virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels,
flags);
break;
case VIR_DOMAIN_DISK_TYPE_DIR:
@@ -14451,17 +14459,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
}
virBufferEscapeString(buf, startupPolicy='%s', startupPolicy);
-if (nseclabels) {
-virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 8);
-for (n = 0; n nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n],
-flags);
-virBufferAdjustIndent(buf, -8);
-virBufferAddLit(buf, /source\n);
-} else {
-virBufferAddLit(buf, /\n);
-}
+virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels,
flags);
break;
default:
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/22] qemu: Refactor qemuTranslatePool source
The function was a mess originally making decisions on volume type
instead of pool type. Refactor it so that it doesn't require a special
formatter function in qemu and use typecasted strings to avoid possible
future omittings.
---
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 74 +++-
src/qemu/qemu_conf.c | 125 ---
src/qemu/qemu_conf.h | 2 +
5 files changed, 96 insertions(+), 107 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4561ccc..a5ef2ca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef {
char *volume; /* volume name */
int voltype; /* enum virStorageVolType, internal only */
int pooltype; /* enum virStoragePoolType, internal only */
+int actualtype; /* enum virDomainDiskType, internal only */
int mode; /* enum virDomainDiskSourcePoolMode */
};
typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 205fe56..aeb3568 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -693,6 +693,7 @@ virStoragePoolSourceFree;
virStoragePoolSourceListFormat;
virStoragePoolSourceListNewSource;
virStoragePoolTypeFromString;
+virStoragePoolTypeToString;
virStorageVolDefFindByKey;
virStorageVolDefFindByName;
virStorageVolDefFindByPath;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8dc7e43..2a4390e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3775,67 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn,
virDomainDiskDefPtr disk, virBufferPtr op
return 0;
}
-static int
-qemuBuildVolumeString(virConnectPtr conn,
- virDomainDiskDefPtr disk,
- virBufferPtr opt)
-{
-int ret = -1;
-
-switch (disk-srcpool-voltype) {
-case VIR_STORAGE_VOL_DIR:
-if (!disk-readonly) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(cannot create virtual FAT disks in read-write
mode));
-goto cleanup;
-}
-if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
-virBufferEscape(opt, ',', ,, file=fat:floppy:%s,, disk-src);
-else
-virBufferEscape(opt, ',', ,, file=fat:%s,, disk-src);
-break;
-case VIR_STORAGE_VOL_BLOCK:
-if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(tray status 'open' is invalid for
- block type volume));
-goto cleanup;
-}
-if (disk-srcpool-pooltype == VIR_STORAGE_POOL_ISCSI) {
-if (disk-srcpool-mode ==
-VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
-if (qemuBuildISCSIString(conn, disk, opt) 0)
-goto cleanup;
-virBufferAddChar(opt, ',');
-} else if (disk-srcpool-mode ==
- VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
-virBufferEscape(opt, ',', ,, file=%s,, disk-src);
-}
-} else {
-virBufferEscape(opt, ',', ,, file=%s,, disk-src);
-}
-break;
-case VIR_STORAGE_VOL_FILE:
-if (disk-auth.username) {
-if (qemuBuildISCSIString(conn, disk, opt) 0)
-goto cleanup;
-virBufferAddChar(opt, ',');
-} else {
-virBufferEscape(opt, ',', ,, file=%s,, disk-src);
-}
-break;
-case VIR_STORAGE_VOL_NETWORK:
-/* Keep the compiler quiet, qemuTranslateDiskSourcePool already
- * reported the unsupported error.
- */
-break;
-}
-
-ret = 0;
-
-cleanup:
-return ret;
-}
char *
qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3849,6 +3788,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainDiskGeometryTransTypeToString(disk-geometry.trans);
int idx = virDiskNameToIndex(disk-dst);
int busid = -1, unitid = -1;
+int actualType = qemuDiskGetActualType(disk);
if (idx 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3932,12 +3872,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
/* disk-src is NULL when we use nbd disks */
if ((disk-src ||
-(disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK
+(actualType == VIR_DOMAIN_DISK_TYPE_NETWORK
disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD))
!((disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM)
disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
-if (disk-type == VIR_DOMAIN_DISK_TYPE_DIR) {
+
+if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) {
/* QEMU only supports magic FAT format for now */
[libvirt] [PATCH 07/22] qemu: Simplify call pattern of qemuBuildDriveURIString
Automatically assign secret type from the disk source definition and
pull in adding of the comma. Then update callers to keep generated
output the same.
---
src/qemu/qemu_command.c | 34 +-
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 91b1bd2..2bb9b79 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3699,9 +3699,7 @@ cleanup:
static int
qemuBuildDriveURIString(virConnectPtr conn,
virDomainDiskDefPtr disk,
-virBufferPtr opt,
-int protocol,
-virSecretUsageType secretUsageType)
+virBufferPtr opt)
{
char *secret = NULL;
char *builturi = NULL;
@@ -3709,20 +3707,22 @@ qemuBuildDriveURIString(virConnectPtr conn,
virBufferAddLit(opt, file=);
-if (disk-auth.username secretUsageType != VIR_SECRET_USAGE_TYPE_NONE) {
+if (disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI
+disk-auth.username) {
/* Get the secret string using the virDomainDiskDef */
if (!(secret = qemuGetSecretString(conn,
-
virDomainDiskProtocolTypeToString(protocol),
+
virDomainDiskProtocolTypeToString(disk-protocol),
false,
disk-auth.secretType,
disk-auth.username,
disk-auth.secret.uuid,
disk-auth.secret.usage,
- secretUsageType)))
+ VIR_SECRET_USAGE_TYPE_ISCSI)))
goto cleanup;
}
-if (!(builturi = qemuBuildNetworkDriveURI(protocol,
+
+if (!(builturi = qemuBuildNetworkDriveURI(disk-protocol,
disk-src,
disk-nhosts,
disk-hosts,
@@ -3731,6 +3731,7 @@ qemuBuildDriveURIString(virConnectPtr conn,
goto cleanup;
virBufferEscape(opt, ',', ,, %s, builturi);
+virBufferAddChar(opt, ',');
ret = 0;
@@ -3760,9 +3761,7 @@ qemuBuildNBDString(virConnectPtr conn,
virDomainDiskDefPtr disk, virBufferPtr op
!disk-hosts-name)
|| (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX
disk-hosts-socket disk-hosts-socket[0] != '/'))
-return qemuBuildDriveURIString(conn, disk, opt,
- VIR_DOMAIN_DISK_PROTOCOL_NBD,
- VIR_SECRET_USAGE_TYPE_NONE);
+return qemuBuildDriveURIString(conn, disk, opt);
virBufferAddLit(opt, file=nbd:);
@@ -3792,6 +3791,8 @@ qemuBuildNBDString(virConnectPtr conn,
virDomainDiskDefPtr disk, virBufferPtr op
if (disk-src)
virBufferEscape(opt, ',', ,, :exportname=%s, disk-src);
+virBufferAddChar(opt, ',');
+
return 0;
}
@@ -3921,7 +3922,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
case VIR_DOMAIN_DISK_PROTOCOL_NBD:
if (qemuBuildNBDString(conn, disk, opt) 0)
goto error;
-virBufferAddChar(opt, ',');
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
virBufferAddLit(opt, file=);
@@ -3929,19 +3929,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
goto error;
virBufferAddChar(opt, ',');
break;
+
case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
-if (qemuBuildDriveURIString(conn, disk, opt,
-VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
-VIR_SECRET_USAGE_TYPE_NONE) 0)
-goto error;
-virBufferAddChar(opt, ',');
-break;
case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
-if (qemuBuildDriveURIString(conn, disk, opt,
-VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
-VIR_SECRET_USAGE_TYPE_ISCSI) 0)
+if (qemuBuildDriveURIString(conn, disk, opt) 0)
goto error;
-virBufferAddChar(opt, ',');
break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 21/22] conf: Add helper do clear disk source authentication struct
Add virDomainDiskAuthClear to help cleaning out the struct in other
places too.
---
src/conf/domain_conf.c | 17 ++---
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b272cb1..bb11011 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1201,12 +1201,9 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def-driverName);
virStorageFileFreeMetadata(def-backingChain);
VIR_FREE(def-mirror);
-VIR_FREE(def-auth.username);
VIR_FREE(def-wwn);
VIR_FREE(def-vendor);
VIR_FREE(def-product);
-if (def-auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
-VIR_FREE(def-auth.secret.usage);
virStorageEncryptionFree(def-encryption);
virDomainDeviceInfoClear(def-info);
@@ -1217,10 +1214,24 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
}
virDomainDiskHostDefFree(def-nhosts, def-hosts);
+virDomainDiskAuthClear(def);
VIR_FREE(def);
}
+
+void
+virDomainDiskAuthClear(virDomainDiskDefPtr def)
+{
+VIR_FREE(def-auth.username);
+
+if (def-auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
+VIR_FREE(def-auth.secret.usage);
+
+def-auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_NONE;
+}
+
+
void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def)
{
if (!def)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ee018f0..4934911 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2210,6 +2210,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr
def);
void virDomainInputDefFree(virDomainInputDefPtr def);
void virDomainDiskDefFree(virDomainDiskDefPtr def);
void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
+void virDomainDiskAuthClear(virDomainDiskDefPtr def);
void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def);
void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts);
virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f952a12..f8e774c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -180,6 +180,7 @@ virDomainDeviceFindControllerModel;
virDomainDeviceInfoCopy;
virDomainDeviceInfoIterate;
virDomainDeviceTypeToString;
+virDomainDiskAuthClear;
virDomainDiskBusTypeToString;
virDomainDiskCacheTypeFromString;
virDomainDiskCacheTypeToString;
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/22] qemu: Use qemuBuildNetworkDriveURI to handle http/ftp and friends
Prepare the function to integrate other protocols and start folding
other network protocols into a common place.
---
src/qemu/qemu_command.c | 199 +---
1 file changed, 122 insertions(+), 77 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2bb9b79..b1c7803 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3627,6 +3627,61 @@ error:
}
+static int
+qemuNetworkDriveGetPort(int protocol,
+const char *port)
+{
+int ret = 0;
+
+if (port) {
+if (virStrToLong_i(port, NULL, 10, ret) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(failed to parse port number '%s'),
+ port);
+return -1;
+}
+
+return ret;
+}
+
+switch ((enum virDomainDiskProtocol) protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+return 80;
+
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+return 443;
+
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+return 21;
+
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+return 990;
+
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+return 69;
+
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+return 7000;
+
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+return 10809;
+
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+/* no default port specified */
+return 0;
+
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+case VIR_DOMAIN_DISK_PROTOCOL_LAST:
+/* not aplicable */
+return -1;
+}
+
+return -1;
+}
+
+
+
char *
qemuBuildNetworkDriveURI(int protocol,
const char *src,
@@ -3638,56 +3693,74 @@ qemuBuildNetworkDriveURI(int protocol,
char *ret = NULL;
virURIPtr uri = NULL;
-if (nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(protocol '%s' accepts only one host),
- virDomainDiskProtocolTypeToString(protocol));
-return NULL;
-}
-
-if (VIR_ALLOC(uri) 0)
-return NULL;
+switch ((enum virDomainDiskProtocol) protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+if (nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(protocol '%s' accepts only one host),
+ virDomainDiskProtocolTypeToString(protocol));
+goto cleanup;
+}
-if (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
-if (VIR_STRDUP(uri-scheme,
virDomainDiskProtocolTypeToString(protocol)) 0)
-goto cleanup;
-} else {
-if (virAsprintf(uri-scheme, %s+%s,
-virDomainDiskProtocolTypeToString(protocol),
-
virDomainDiskProtocolTransportTypeToString(hosts-transport)) 0)
-goto cleanup;
-}
+if (VIR_ALLOC(uri) 0)
+goto cleanup;
-if (src
-virAsprintf(uri-path, /%s, src) 0)
-goto cleanup;
+if (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
+if (VIR_STRDUP(uri-scheme,
+ virDomainDiskProtocolTypeToString(protocol))
0)
+goto cleanup;
+} else {
+if (virAsprintf(uri-scheme, %s+%s,
+virDomainDiskProtocolTypeToString(protocol),
+
virDomainDiskProtocolTransportTypeToString(hosts-transport)) 0)
+goto cleanup;
+}
-if (hosts-port
-virStrToLong_i(hosts-port, NULL, 10, uri-port) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(failed to parse port number '%s'),
- hosts-port);
-goto cleanup;
-}
+if ((uri-port = qemuNetworkDriveGetPort(protocol, hosts-port))
0)
+goto cleanup;
-if (hosts-socket
-virAsprintf(uri-query, socket=%s, hosts-socket) 0)
-goto cleanup;
+if (src
+virAsprintf(uri-path, %s%s,
+src[0] == '/' ? : /,
+src) 0)
+goto cleanup;
-if (username) {
-if (secret) {
-if (virAsprintf(uri-user, %s:%s, username, secret) 0)
+if (hosts-socket
+virAsprintf(uri-query, socket=%s, hosts-socket) 0)
goto cleanup;
-} else {
-
[libvirt] [PATCH 02/22] test: Implement fake storage pool driver in qemuxml2argv test
To support testing of volume disk backing, we need to implement a few
disk driver backend functions.
The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml
as XML files for pool definitions and volume names are in format
VOL_TYPE+VOL_PATH. By default type block is assumed (for iSCSI test
compatibility).
The choice of this approach along with implemented functions was made so
that disk type='volume' can be tested in the xml2argv test.
---
tests/qemuxml2argvtest.c | 162 +++
1 file changed, 162 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a290062..07640db 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -18,6 +18,7 @@
# include qemu/qemu_command.h
# include qemu/qemu_domain.h
# include datatypes.h
+# include conf/storage_conf.h
# include cpu/cpu_map.h
# include virstring.h
@@ -75,6 +76,161 @@ static virSecretDriver fakeSecretDriver = {
.secretUndefine = NULL,
};
+
+# define STORAGE_POOL_XML_PATH storagepoolxml2xmlout/
+static const unsigned char fakeUUID[VIR_UUID_BUFLEN] = fakeuuid;
+
+static virStoragePoolPtr
+fakeStoragePoolLookupByName(virConnectPtr conn,
+const char *name)
+{
+char *xmlpath = NULL;
+virStoragePoolPtr ret = NULL;
+
+if (STRNEQ(name, inactive)) {
+if (virAsprintf(xmlpath, %s%s.xml,
+STORAGE_POOL_XML_PATH,
+name) 0)
+return NULL;
+
+if (!virFileExists(xmlpath)) {
+virReportError(VIR_ERR_NO_STORAGE_POOL,
+ File '%s' not found, xmlpath);
+goto cleanup;
+}
+}
+
+ret = virGetStoragePool(conn, name, fakeUUID, NULL, NULL);
+
+cleanup:
+VIR_FREE(xmlpath);
+return ret;
+}
+
+
+static virStorageVolPtr
+fakeStorageVolLookupByName(virStoragePoolPtr pool,
+ const char *name)
+{
+char **volinfo = NULL;
+virStorageVolPtr ret = NULL;
+
+if (STREQ(pool-name, inactive)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ storage pool '%s' is not active, pool-name);
+return NULL;
+}
+
+if (STREQ(name, nonexistent)) {
+virReportError(VIR_ERR_NO_STORAGE_VOL,
+ no storage vol with matching name '%s', name);
+return NULL;
+}
+
+if (!strchr(name, '+'))
+goto fallback;
+
+if (!(volinfo = virStringSplit(name, +, 2)))
+return NULL;
+
+if (!volinfo[1])
+goto fallback;
+
+ret = virGetStorageVol(pool-conn, pool-name, volinfo[1], volinfo[0],
+ NULL, NULL);
+
+cleanup:
+virStringFreeList(volinfo);
+return ret;
+
+fallback:
+ret = virGetStorageVol(pool-conn, pool-name, name, block, NULL, NULL);
+goto cleanup;
+}
+
+static int
+fakeStorageVolGetInfo(virStorageVolPtr vol,
+ virStorageVolInfoPtr info)
+{
+memset(info, 0, sizeof(info));
+
+info-type = virStorageVolTypeFromString(vol-key);
+
+if (info-type 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Invalid volume type '%s', vol-key);
+return -1;
+}
+
+return 0;
+}
+
+
+static char *
+fakeStorageVolGetPath(virStorageVolPtr vol)
+{
+char *ret = NULL;
+
+ignore_value(virAsprintf(ret, /some/%s/device/%s, vol-key, vol-name));
+
+return ret;
+}
+
+
+static char *
+fakeStoragePoolDumpXML(virStoragePoolPtr pool,
+ unsigned int flags_unused ATTRIBUTE_UNUSED)
+{
+char *xmlpath = NULL;
+char *xmlbuf = NULL;
+
+if (STREQ(pool-name, inactive)) {
+virReportError(VIR_ERR_NO_STORAGE_POOL, NULL);
+return NULL;
+}
+
+if (virAsprintf(xmlpath, %s%s.xml,
+STORAGE_POOL_XML_PATH,
+pool-name) 0)
+return NULL;
+
+if (virtTestLoadFile(xmlpath, xmlbuf) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ failed to load XML file '%s',
+ xmlpath);
+goto cleanup;
+}
+
+cleanup:
+VIR_FREE(xmlpath);
+
+return xmlbuf;
+}
+
+static int
+fakeStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED)
+{
+return 0;
+}
+
+static int
+fakeStoragePoolIsActive(virStoragePoolPtr pool ATTRIBUTE_UNUSED)
+{
+return 1;
+}
+
+
+static virStorageDriver fakeStorageDriver = {
+.name = fake_storage,
+.storageClose = fakeStorageClose,
+.storagePoolLookupByName = fakeStoragePoolLookupByName,
+.storageVolLookupByName = fakeStorageVolLookupByName,
+.storagePoolGetXMLDesc = fakeStoragePoolDumpXML,
+.storageVolGetPath = fakeStorageVolGetPath,
+.storageVolGetInfo = fakeStorageVolGetInfo,
+.storagePoolIsActive = fakeStoragePoolIsActive,
+};
+
typedef enum {
FLAG_EXPECT_ERROR = 1 0,
FLAG_EXPECT_FAILURE = 1 1,
@@ -103,6 +259,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
[libvirt] [PATCH 22/22] qemu: Clear old translated pool source
Clear the old data to avoid leaking it when attempting to re-translate a
pool on the same domain object.
---
src/qemu/qemu_conf.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e098c13..1192d23 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1360,6 +1360,10 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
goto cleanup;
}
+VIR_FREE(def-src);
+virDomainDiskHostDefFree(def-nhosts, def-hosts);
+virDomainDiskAuthClear(def);
+
switch ((enum virStoragePoolType) pooldef-type) {
case VIR_STORAGE_POOL_DIR:
case VIR_STORAGE_POOL_FS:
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 20/22] qemu: snapshot: Detect internal snapshots also for sheepdog and RBD
When doing an internal snapshot on a VM with sheepdog or RBD disks we
would not set a flag to mark the domain is using internal snapshots and
might end up creating a mixed snapshot. Move the setting of the variable
to avoid this problem.
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8a1eefd..d2dbaf5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11764,6 +11764,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
virDomainSnapshotDefPtr def,
switch (disk-snapshot) {
case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
+found_internal = true;
+
if (def-state != VIR_DOMAIN_DISK_SNAPSHOT
dom_disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK
(dom_disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG ||
@@ -11787,7 +11789,6 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
virDomainSnapshotDefPtr def,
disk-name);
goto cleanup;
}
-found_internal = true;
break;
case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 19/22] conf: Add functions to copy and free network disk source definitions
To simplify operations on virDomainDiskHostDef arrays we will need deep
copy and freeing functions. Add and properly export them.
---
src/conf/domain_conf.c | 55 +---
src/conf/domain_conf.h | 3 +++
src/libvirt_private.syms | 2 ++
3 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 09ea51f..b272cb1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1216,9 +1216,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def-seclabels);
}
-for (i = 0; i def-nhosts; i++)
-virDomainDiskHostDefClear(def-hosts[i]);
-VIR_FREE(def-hosts);
+virDomainDiskHostDefFree(def-nhosts, def-hosts);
VIR_FREE(def);
}
@@ -1233,6 +1231,57 @@ void virDomainDiskHostDefClear(virDomainDiskHostDefPtr
def)
VIR_FREE(def-socket);
}
+
+void
+virDomainDiskHostDefFree(size_t nhosts,
+ virDomainDiskHostDefPtr hosts)
+{
+size_t i;
+
+if (!hosts)
+return;
+
+for (i = 0; i nhosts; i++)
+virDomainDiskHostDefClear(hosts[i]);
+
+VIR_FREE(hosts);
+}
+
+
+virDomainDiskHostDefPtr
+virDomainDiskHostDefCopy(size_t nhosts,
+ virDomainDiskHostDefPtr hosts)
+{
+virDomainDiskHostDefPtr ret = NULL;
+size_t i;
+
+if (VIR_ALLOC_N(ret, nhosts) 0)
+goto error;
+
+for (i = 0; i nhosts; i++) {
+virDomainDiskHostDefPtr src = hosts[i];
+virDomainDiskHostDefPtr dst = ret[i];
+
+dst-transport = src-transport;
+
+if (VIR_STRDUP(dst-name, src-name) 0)
+goto error;
+
+if (VIR_STRDUP(dst-port, src-port) 0)
+goto error;
+
+if (VIR_STRDUP(dst-socket, src-socket) 0)
+goto error;
+}
+
+return ret;
+
+error:
+virDomainDiskHostDefFree(nhosts, ret);
+return NULL;
+}
+
+
void virDomainControllerDefFree(virDomainControllerDefPtr def)
{
if (!def)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e9800a5..ee018f0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2211,6 +2211,9 @@ void virDomainInputDefFree(virDomainInputDefPtr def);
void virDomainDiskDefFree(virDomainDiskDefPtr def);
void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def);
+void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts);
+virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts,
+ virDomainDiskHostDefPtr
hosts);
int virDomainDeviceFindControllerModel(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
int controllerType);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aeb3568..f952a12 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -198,6 +198,8 @@ virDomainDiskFindByBusAndDst;
virDomainDiskGeometryTransTypeFromString;
virDomainDiskGeometryTransTypeToString;
virDomainDiskHostDefClear;
+virDomainDiskHostDefCopy;
+virDomainDiskHostDefFree;
virDomainDiskIndexByName;
virDomainDiskInsert;
virDomainDiskInsertPreAlloced;
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/22] snapshot: conf: Use common parsing and formatting functions for source
Disk source elements for snapshots were using separate code from our
config parser. As snapshots can be stored on more than just regular
files, we will need the universal parser to allow us to expose a variety
of snapshot disk targets. This patch reuses the config parsers and
formatters to do the job.
This initial support only changes the code without any visible XML
change.
---
src/conf/snapshot_conf.c | 70 +---
src/conf/snapshot_conf.h | 1 +
2 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 94a74d2..7258daa 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -128,27 +128,42 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
}
}
-cur = node-children;
-while (cur) {
-if (cur-type == XML_ELEMENT_NODE) {
-if (!def-file
-xmlStrEqual(cur-name, BAD_CAST source)) {
-def-file = virXMLPropString(cur, file);
-} else if (!def-format
- xmlStrEqual(cur-name, BAD_CAST driver)) {
-char *driver = virXMLPropString(cur, type);
-def-format = virStorageFileFormatTypeFromString(driver);
-if (def-format = 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(unknown disk snapshot driver '%s'),
- driver);
-VIR_FREE(driver);
-goto cleanup;
-}
+def-type = -1;
+
+for (cur = node-children; cur; cur = cur-next) {
+if (cur-type != XML_ELEMENT_NODE)
+continue;
+
+if (!def-file
+xmlStrEqual(cur-name, BAD_CAST source)) {
+
+int backingtype = def-type;
+
+if (backingtype 0)
+backingtype = VIR_DOMAIN_DISK_TYPE_FILE;
+
+if (virDomainDiskSourceDefParse(cur,
+backingtype,
+def-file,
+NULL,
+NULL,
+NULL,
+NULL) 0)
+goto cleanup;
+
+} else if (!def-format
+ xmlStrEqual(cur-name, BAD_CAST driver)) {
+char *driver = virXMLPropString(cur, type);
+def-format = virStorageFileFormatTypeFromString(driver);
+if (def-format = 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unknown disk snapshot driver '%s'),
+ driver);
VIR_FREE(driver);
+goto cleanup;
}
+VIR_FREE(driver);
}
-cur = cur-next;
}
if (!def-snapshot (def-file || def-format))
@@ -577,6 +592,8 @@ static void
virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virDomainSnapshotDiskDefPtr disk)
{
+int type = disk-type;
+
if (!disk-name)
return;
@@ -584,6 +601,13 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
if (disk-snapshot 0)
virBufferAsprintf(buf, snapshot='%s',
virDomainSnapshotLocationTypeToString(disk-snapshot));
+
+if (type 0)
+type = VIR_DOMAIN_DISK_TYPE_FILE;
+else
+virBufferAsprintf(buf, type='%s',
+ virDomainDiskTypeToString(type));
+
if (!disk-file disk-format == 0) {
virBufferAddLit(buf, /\n);
return;
@@ -591,12 +615,14 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 6);
if (disk-format 0)
-virBufferEscapeString(buf, driver type='%s'/\n,
+virBufferEscapeString(buf, driver type='%s'/\n,
virStorageFileFormatTypeToString(disk-format));
-virBufferEscapeString(buf, source file='%s'/\n, disk-file);
-virBufferAdjustIndent(buf, -6);
+virDomainDiskSourceDefFormatInternal(buf,
+ type,
+ disk-file,
+ 0, 0, 0, NULL, 0, NULL, NULL, 0);
+
virBufferAddLit(buf, /disk\n);
}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ff3dca2..241d63c 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -51,6 +51,7 @@ struct _virDomainSnapshotDiskDef {
char *name; /* name matching the target dev='...' of the domain */
int index; /* index within snapshot-dom-disks that matches name */
int snapshot; /* enum virDomainSnapshotLocation */
+int type; /* enum virDomainDiskType */
char *file; /* new source file when snapshot is external */
int format; /* enum virStorageFileFormat
[libvirt] [PATCH] Remove obsolete 'tests' makefile target
From: Daniel P. Berrange berra...@redhat.com The 'docs/examples' code was long ago removed and now the python code was gone too, the custom 'tests' makefile target serves no purpopse Signed-off-by: Daniel P. Berrange berra...@redhat.com --- Makefile.am | 3 --- 1 file changed, 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index 957aa9f..d7ddd9d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -71,9 +71,6 @@ rpm: clean check-local: all tests -tests: - @(cd docs/examples ; $(MAKE) MAKEFLAGS+=--silent tests) - cov: clean-cov mkdir $(top_builddir)/coverage $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Return right error code for baselineCPU
On Mon, Nov 25, 2013 at 04:02:37PM +, Daniel P. Berrange wrote:
On Mon, Nov 25, 2013 at 08:55:12AM -0700, Don Dugger wrote:
On Mon, Nov 25, 2013 at 03:48:45PM +, Daniel P. Berrange wrote:
On Mon, Nov 25, 2013 at 08:40:41AM -0700, Don Dugger wrote:
On Mon, Nov 25, 2013 at 10:45:38AM +, Daniel P. Berrange wrote:
On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote:
On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger n0...@n0ano.com wrote:
This Python interface code is returning a -1 on errors for the
`baselineCPU' API. Since this API is supposed to return a pointer
the error return value should really be VIR_PY_NONE.
...
ACK. This is correct. But it obviously changes our API so I'm not
really sure how we should handle this, (e.g. document the API as is
as
note that its broken or fix it).
The implicit expectation with python APIs is that they all raise an
exception if the libvirt call fails. So ACK to this bug fix we
should put it in maint branches.
Much as I hate to raise the issue this assumption is true for pointer
APIs but APIs that return an integer don't raise an exception, they
just return -1. Obviously, changing this behavior would be way too
invasive but documenting this behavior should be done somewhere.
What APIs in particular ? Any API which results in a libvirt error
being set should be translated into an exception in the python. This
is done whether they're APIs returning NULL pointers or -1 ints. If
there are other exceptions to the rule they must be fixed too.
I'm basing this on code inspection so I could be wrong but if you look
at the Python interface code for libvirt_virDomainSetSchedulerParameters
it checks the return value from virDomainSetSchedulerParameters and,
if it is 0, then the Pythong code returns -1, without raising an
exception.
That code is the low-level module (called libvirtmod) which interfaces
to the C library, and is not called directly by applications.
Applications call the main module (called libvirt) which translates
to the lowlevel module. eg in this case it does:
def setSchedulerParametersFlags(self, params, flags=0):
Change the scheduler parameters
ret = libvirtmod.virDomainSetSchedulerParametersFlags(self._o,
params, flags)
if ret == -1: raise libvirtError
('virDomainSetSchedulerParametersFlags() failed', dom=self)
return ret
so the -1 error is turned into an exception for applications.
The flaw the original patch in this thread was fixing is that the low
level libvirtmod was returning NULL, where the high level libvirt
expected to see -1, so the exception translation was missed.
Eric/Daniel-
That's why I distrust code inspection, there's always a hidden gotcha. Tnx
for the clarification.
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
Re: [libvirt] [PATCH] Remove obsolete 'tests' makefile target
On 11/25/2013 09:15 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The 'docs/examples' code was long ago removed and now the python code was gone too, the custom 'tests' makefile target serves no purpopse s/purpopse/purpose/ ACK. -- 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
[libvirt] [PATCHv2] storage: expose volume meta-type in XML
I got annoyed at having to use both 'virsh vol-list $pool --details'
AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated
the volume correctly. Since two-thirds of the data present in
virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(),
this just adds the remaining piece of information, as:
volume type='...'
...
/volume
* docs/formatstorage.html.in: Document new volume type=
* docs/schemas/storagevol.rng (vol): Add it to RelaxNG.
* src/conf/storage_conf.h (virStorageVolTypeToString): Declare.
* src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output
the metatype.
(virStorageVolDefParseXML): Parse it, for unit tests.
* tests/storagevolxml2xmlout/vol-*.xml: Update tests to match.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v1: https://www.redhat.com/archives/libvir-list/2013-November/msg00955.html
Since then, delay 'network-dir' until later in gluster series,
change XML to volume type='file' instead of volumetypefile/type,
and use 1.2.0 instead of 1.1.5
docs/formatstorage.html.in | 10 +++---
docs/schemas/storagevol.rng| 10 ++
src/conf/storage_conf.c| 19 ++-
src/conf/storage_conf.h| 1 +
tests/storagevolxml2xmlin/vol-logical-backing.xml | 2 +-
tests/storagevolxml2xmlin/vol-logical.xml | 2 +-
tests/storagevolxml2xmlin/vol-partition.xml| 2 +-
tests/storagevolxml2xmlin/vol-sheepdog.xml | 2 +-
tests/storagevolxml2xmlout/vol-file-backing.xml| 2 +-
tests/storagevolxml2xmlout/vol-file-naming.xml | 2 +-
tests/storagevolxml2xmlout/vol-file.xml| 2 +-
tests/storagevolxml2xmlout/vol-logical-backing.xml | 2 +-
tests/storagevolxml2xmlout/vol-logical.xml | 2 +-
tests/storagevolxml2xmlout/vol-partition.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2.xml | 2 +-
tests/storagevolxml2xmlout/vol-sheepdog.xml| 2 +-
20 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 90eeaa3..c90d7b1 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -283,14 +283,18 @@
h2a name=StorageVolStorage volume XML/a/h2
p
- A storage volume will be either a file or a device node.
- The storage volume XML format is available span class=sincesince
0.4.1/span
+ A storage volume will generally be either a file or a device
+ node; span class=sincesince 1.2.0/span, an optional
+ output-only attribute codetype/code lists the actual type
+ (file, block, dir, or network), which is also available
+ from codevirStorageVolGetInfo()/code. The storage volume
+ XML format is available span class=sincesince 0.4.1/span
/p
h3a name=StorageVolFirstGeneral metadata/a/h3
pre
- lt;volumegt;
+ lt;volume type='file'gt;
lt;namegt;sparse.imglt;/namegt;
lt;keygt;/var/lib/xen/images/sparse.imglt;/keygt;
lt;allocationgt;0lt;/allocationgt;
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index e79bc35..f8081d9 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -13,6 +13,16 @@
define name='vol'
element name='volume'
+ optional
+attribute name='type'
+ choice
+valuefile/value
+valueblock/value
+valuedir/value
+valuenetwork/value
+ /choice
+/attribute
+ /optional
interleave
element name='name'
ref name='volName'/
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8b378c2..0cd80c3 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -51,6 +51,10 @@
#define DEFAULT_POOL_PERM_MODE 0755
#define DEFAULT_VOL_PERM_MODE 0600
+VIR_ENUM_IMPL(virStorageVol,
+ VIR_STORAGE_VOL_LAST,
+ file, block, dir, network)
+
VIR_ENUM_IMPL(virStoragePool,
VIR_STORAGE_POOL_LAST,
dir, fs, netfs,
@@ -1253,6 +1257,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
{
virStorageVolDefPtr ret;
virStorageVolOptionsPtr options;
+char *type = NULL;
char *allocation = NULL;
char *capacity = NULL;
char *unit = NULL;
@@ -1278,6 +1283,16 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
/* Normally generated by pool refresh, but useful for unit tests */
ret-key = virXPathString(string(./key), ctxt);
+/* Technically overridden by pool refresh, but useful for unit tests */
+type = virXPathString(string(./@type), ctxt);
+if (type) {
+if ((ret-type =
Re: [libvirt] [PATCHv2] storage: expose volume meta-type in XML
On Mon, Nov 25, 2013 at 09:56:48AM -0700, Eric Blake wrote: I got annoyed at having to use both 'virsh vol-list $pool --details' AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated the volume correctly. Since two-thirds of the data present in virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(), this just adds the remaining piece of information, as: volume type='...' ... /volume * docs/formatstorage.html.in: Document new volume type= * docs/schemas/storagevol.rng (vol): Add it to RelaxNG. * src/conf/storage_conf.h (virStorageVolTypeToString): Declare. * src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output the metatype. (virStorageVolDefParseXML): Parse it, for unit tests. * tests/storagevolxml2xmlout/vol-*.xml: Update tests to match. Signed-off-by: Eric Blake ebl...@redhat.com 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
[libvirt] [PATCH libvirt-python] Update README file contents and add HACKING file
From: Daniel P. Berrange berra...@redhat.com The previous README file from the python code is more like a HACKING file. Rename it and update the content. Then add a basic README file Signed-off-by: Daniel P. Berrange berra...@redhat.com --- HACKING | 37 + README | 41 + 2 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 HACKING diff --git a/HACKING b/HACKING new file mode 100644 index 000..b0f037f --- /dev/null +++ b/HACKING @@ -0,0 +1,37 @@ +libvirt Python Bindings Hacking +=== + +Most of the libvirt python binding code is automatically generated +using the script generator.py, and the API description that the +libvirt library installs into /usr/share (or wherever the following +command says. + + $ pkg-config --variable libvirt_api libvirt + /usr/share/libvirt/api/libvirt-api.xml + + +Some of the API descriptions in the primary XML files are not directlry +usable by the code generator. Thus there are overrides in + + - libvirt-override-api.xml + - libvirt-qemu-override-api.xml + - libvirt-lxc-override-api.xml + +For stuff which the genrator can't cope with at all there are some +hand written source files + + - libvirt-override.c - low level binding to libvirt.so + - libvirt-qemu-override.c - low level binding to libvirt-qemu.so + - libvirt-lxc-override.c - low level binding to libvirt-lxc.so + + - libvirt-override.py - high level overrides in the global namespace + - libvirt-override-virConnect.py - high level overrides in + the virConnect class + - libvirt-override-virDomain.py - high level overrides in + the virDomain class + - libvirt-override-virDomainSnapshot.py - high level overrides in + the virDomainSnapshot class + - libvirt-override-virStoragePool.py - high level overrides in + the virStoragePool class + - libvirt-override-virStream.py - high level overrides in + the virStream class diff --git a/README b/README index 02d4cc4..cadd2e4 100644 --- a/README +++ b/README @@ -1,27 +1,28 @@ -libvirt Python Bindings README -== + Libvirt Python Binding README + = -Most of the libvirt python binding code is automatically generated -using the script generator.py, and the API description from -docs/libvirt-api.xml +This package provides a python binding to the libvirt.so, +libvirt-qemu.so and libvirt-lxc.so library APIs. +It is written to build against any version of libvirt that +is 0.9.11 or newer. -Manually written files: +This code is distributed under the terms of the LGPL version +2 or later. - - libvirt-override.c: methods where the C binding needs to be hand crafted - - libvirt-override.py: global methods where the C and python bindings have different args - - libvirt-override-api.xml: methods where the auto-extracted API docs are not - suitable for python auto-generator. Overriding this if the method is going - into libvirt-override.c, but we still want auto-generated libvirt-override.py - - libvirt-override-virConnect.py: virConnect class methods - - typewrappers.h,.c: Python object wrappers for each libvirt C object +The module can be built by following the normal python module +build processs + python setup.py build + sudo python setup.py install -Auto-generated files: +or to install as non-root - - libvirt.py: The main python binding. Comprises auto-generated code, along -with contents from libvirt-override.py and libvirt-override-virConnect.py - - libvirt.c, libvirt.h: The C glue layer for the python binding. Comprises -auto-generated code, along with libvirt-override.c - - libvirt-export.c: List of auto-generated C methods, included into -the libvirt-override.c method table + python setup.py build + python setup.py install --user + + +Patches for this code should be sent to the main libvirt +development mailing list + + http://libvirt.org/contact.html#email -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] storage: expose volume meta-type in XML
On 11/25/2013 10:06 AM, Daniel P. Berrange wrote: On Mon, Nov 25, 2013 at 09:56:48AM -0700, Eric Blake wrote: I got annoyed at having to use both 'virsh vol-list $pool --details' AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated the volume correctly. Since two-thirds of the data present in virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(), this just adds the remaining piece of information, as: volume type='...' ... /volume * docs/formatstorage.html.in: Document new volume type= * docs/schemas/storagevol.rng (vol): Add it to RelaxNG. * src/conf/storage_conf.h (virStorageVolTypeToString): Declare. * src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output the metatype. (virStorageVolDefParseXML): Parse it, for unit tests. * tests/storagevolxml2xmlout/vol-*.xml: Update tests to match. Signed-off-by: Eric Blake ebl...@redhat.com ACK Thanks; will push shortly. -- 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] [PATCHv2] storage: expose volume meta-type in XML
On 11/25/2013 11:56 AM, Eric Blake wrote:
I got annoyed at having to use both 'virsh vol-list $pool --details'
AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated
the volume correctly. Since two-thirds of the data present in
virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(),
this just adds the remaining piece of information, as:
volume type='...'
...
/volume
* docs/formatstorage.html.in: Document new volume type=
* docs/schemas/storagevol.rng (vol): Add it to RelaxNG.
* src/conf/storage_conf.h (virStorageVolTypeToString): Declare.
* src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output
the metatype.
(virStorageVolDefParseXML): Parse it, for unit tests.
* tests/storagevolxml2xmlout/vol-*.xml: Update tests to match.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v1: https://www.redhat.com/archives/libvir-list/2013-November/msg00955.html
Since then, delay 'network-dir' until later in gluster series,
change XML to volume type='file' instead of volumetypefile/type,
and use 1.2.0 instead of 1.1.5
docs/formatstorage.html.in | 10 +++---
docs/schemas/storagevol.rng| 10 ++
src/conf/storage_conf.c| 19 ++-
src/conf/storage_conf.h| 1 +
tests/storagevolxml2xmlin/vol-logical-backing.xml | 2 +-
tests/storagevolxml2xmlin/vol-logical.xml | 2 +-
tests/storagevolxml2xmlin/vol-partition.xml| 2 +-
tests/storagevolxml2xmlin/vol-sheepdog.xml | 2 +-
tests/storagevolxml2xmlout/vol-file-backing.xml| 2 +-
tests/storagevolxml2xmlout/vol-file-naming.xml | 2 +-
tests/storagevolxml2xmlout/vol-file.xml| 2 +-
tests/storagevolxml2xmlout/vol-logical-backing.xml | 2 +-
tests/storagevolxml2xmlout/vol-logical.xml | 2 +-
tests/storagevolxml2xmlout/vol-partition.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml | 2 +-
tests/storagevolxml2xmlout/vol-qcow2.xml | 2 +-
tests/storagevolxml2xmlout/vol-sheepdog.xml| 2 +-
20 files changed, 52 insertions(+), 20 deletions(-)
You and Peter seem to be touching the same code in storage_conf.c:
https://www.redhat.com/archives/libvir-list/2013-November/msg01030.html
Although added virStorageVol was added in a different location...
That'll make the 3 way merge interesting...
Also, he seems to have also touched libvirt_private.syms too...
John
...snip...
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8b378c2..0cd80c3 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -51,6 +51,10 @@
#define DEFAULT_POOL_PERM_MODE 0755
#define DEFAULT_VOL_PERM_MODE 0600
+VIR_ENUM_IMPL(virStorageVol,
+ VIR_STORAGE_VOL_LAST,
+ file, block, dir, network)
+
VIR_ENUM_IMPL(virStoragePool,
VIR_STORAGE_POOL_LAST,
dir, fs, netfs,
@@ -1253,6 +1257,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
{
virStorageVolDefPtr ret;
virStorageVolOptionsPtr options;
+char *type = NULL;
char *allocation = NULL;
char *capacity = NULL;
char *unit = NULL;
@@ -1278,6 +1283,16 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
/* Normally generated by pool refresh, but useful for unit tests */
ret-key = virXPathString(string(./key), ctxt);
+/* Technically overridden by pool refresh, but useful for unit tests */
+type = virXPathString(string(./@type), ctxt);
+if (type) {
+if ((ret-type = virStorageVolTypeFromString(type)) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(unknown volume type '%s'), type);
+goto error;
+}
+}
+
capacity = virXPathString(string(./capacity), ctxt);
unit = virXPathString(string(./capacity/@unit), ctxt);
if (capacity == NULL) {
@@ -1394,6 +1409,7 @@ cleanup:
VIR_FREE(allocation);
VIR_FREE(capacity);
VIR_FREE(unit);
+VIR_FREE(type);
return ret;
error:
@@ -1563,7 +1579,8 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
if (options == NULL)
return NULL;
-virBufferAddLit(buf, volume\n);
+virBufferAsprintf(buf, volume type='%s'\n,
+ virStorageVolTypeToString(def-type));
virBufferEscapeString(buf, name%s/name\n, def-name);
virBufferEscapeString(buf, key%s/key\n, def-key);
virBufferAddLit(buf, source\n);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index f062bd8..c4dd403 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -116,6 +116,7 @@ struct _virStorageVolDefList {
[libvirt] [PATCHv4 0/5] Handling of undefine and redefine snapshots with VirtualBox 4.2
Hi, This is a serie of patches in order to support undefining and redefining snapshots with VirtualBox 4.2. The serie of patches is rather big, and adds among other things some utility functions unrelated to VirtualBox in patches 1 2. The code review could be done in several parts: e.g. patches 1 2 separately to validate the utility functions. The VirtualBox API provides only high level operations to manipulate snapshots, so it not possible to support flags like VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY with only API calls. Following an IRC talk with Eric Blake, the decision was taken to emulate these behaviours by manipulating directly the .vbox XML files. The first two patches are some util methods for handling uuid and strings that will be used after. The third patch brings more details in the snapshot XML returned by libvirt. We will need those modifications in order to redefine the snapshots. The fourth patch brings the support of the VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flags in virDomainSnapshotCreateXML. The fifth and last patch brings the support of the VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY flag in virDomainSnapshotDelete. The patches are only tested with Virtualbox 4.2 but the code is compliant with Virtualbox 4.3 API. Regards, Manuel VIVES V4: * The code is compliant with Virtualbox 4.3 API * Some minor modifications in order to satisfy make syntax-check V3: * Use of STREQ_NULLABLE instead of STREQ in one case * Fix the method for finding uuids according to Ján Tomko review V2: * Fix a licence problem with the method for string replacement Manuel VIVES (5): viruuid.h/c: Util method for finding uuid patterns in some strings virstring.h/c: Util method for making some find and replace in strings vbox_tmpl.c: Better XML description for snapshots vbox_tmpl.c: Patch for redefining snapshots vbox_tmpl.c: Add methods for undefining snapshots po/POTFILES.in |1 + src/conf/domain_conf.c |2 +- src/libvirt_private.syms |2 + src/util/virstring.c | 48 ++ src/util/virstring.h |2 + src/util/viruuid.c | 81 ++ src/util/viruuid.h |1 + src/vbox/vbox_tmpl.c | 1854 +++--- 8 files changed, 1879 insertions(+), 112 deletions(-) -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 2/5] virstring.h/c: Util method for making some find and replace in strings
---
src/libvirt_private.syms |1 +
src/util/virstring.c | 48 ++
src/util/virstring.h |2 ++
3 files changed, 51 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 99cc32a..b761fb6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1750,6 +1750,7 @@ virStringListLength;
virStringSplit;
virStrncpy;
virStrndup;
+virStrReplace;
virStrToDouble;
virStrToLong_i;
virStrToLong_l;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index d11db5c..a30a4ef 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -616,3 +616,51 @@ size_t virStringListLength(char **strings)
return i;
}
+
+/*
+ virStrReplace(haystack, oldneedle, newneedle) --
+ Search haystack and replace all occurences of oldneedle with newneedle.
+ Return a string with all the replacements in case of success, NULL in case
+ of failure
+*/
+char *
+virStrReplace(char *haystack,
+ const char *oldneedle, const char *newneedle)
+{
+char *ret;
+size_t i, count = 0;
+size_t newneedle_len = strlen(newneedle);
+size_t oldneedle_len = strlen(oldneedle);
+size_t totalLength = 0;
+if (strlen(oldneedle) == 0) {
+ignore_value(VIR_STRDUP(ret, haystack));
+goto cleanup;
+}
+
+for (i = 0; haystack[i] != '\0'; i++) {
+if (strstr(haystack[i], oldneedle) == haystack[i]) {
+count++;
+i += oldneedle_len - 1;
+}
+}
+if (VIR_ALLOC_N(ret, (i + count * (newneedle_len - oldneedle_len))) 0) {
+ret = NULL;
+goto cleanup;
+}
+totalLength = sizeof(char *)*(i + count * (newneedle_len - oldneedle_len));
+i = 0;
+while (*haystack) {
+if (strstr(haystack, oldneedle) == haystack) {
+if (virStrcpy(ret[i], newneedle, totalLength) == NULL) {
+ret = NULL;
+goto cleanup;
+}
+i += newneedle_len;
+haystack += oldneedle_len;
+} else
+ret[i++] = *haystack++;
+}
+ret[i] = '\0';
+cleanup:
+return ret;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index b390150..90522bd 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -223,4 +223,6 @@ size_t virStringListLength(char **strings);
virAsprintfInternal(false, 0, NULL, NULL, 0, \
strp, __VA_ARGS__)
+char * virStrReplace(char *haystack, const char *oldneedle, const char
*newneedle);
+
#endif /* __VIR_STRING_H__ */
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 4/5] vbox_tmpl.c: Patch for redefining snapshots
The snapshots are saved in xml files, and then can be redefined
---
src/vbox/vbox_tmpl.c | 852 +-
1 file changed, 844 insertions(+), 8 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index e05ed97..23f8aab 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -61,6 +61,7 @@
#include virstring.h
#include virtime.h
#include virutil.h
+#include dirname.h
/* This one changes from version to version. */
#if VBOX_API_VERSION == 2002
@@ -276,6 +277,12 @@ static vboxGlobalData *g_pVBoxGlobalData = NULL;
#endif /* VBOX_API_VERSION = 4000 */
+/*This error is a bit specific
+ *In the VBOX API it is named E_ACCESSDENIED
+ *It is returned when the called object is not ready. In
+ *particular when we do any call on a disk which has been closed
+*/
+#define VBOX_E_ACCESSDENIED 0x80070005
#define reportInternalErrorIfNS_FAILED(message) \
if (NS_FAILED(rc)) { \
virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(message)); \
@@ -286,6 +293,8 @@ static vboxGlobalData *g_pVBoxGlobalData = NULL;
static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml);
static int vboxDomainCreate(virDomainPtr dom);
static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags);
+static virStorageVolPtr vboxStorageVolLookupByPath(virConnectPtr conn, const
char *path);
+static int vboxStorageDeleteOrClose(virStorageVolPtr vol, unsigned int flags,
unsigned int flagDeleteOrClose);
static void vboxDriverLock(vboxGlobalData *data) {
virMutexLock(data-lock);
}
@@ -5946,6 +5955,827 @@ cleanup:
return snapshot;
}
+#if VBOX_API_VERSION =4002
+static void
+vboxSnapshotXmlRetrieveSnapshotNodeByName(xmlNodePtr a_node,
+ const char *name,
+ xmlNodePtr *snap_node)
+{
+xmlNodePtr cur_node = NULL;
+
+for (cur_node = a_node; cur_node; cur_node = cur_node-next) {
+if (cur_node-type == XML_ELEMENT_NODE) {
+if (!xmlStrcmp(cur_node-name, (const xmlChar *) Snapshot)
+STREQ(virXMLPropString(cur_node, name), name)) {
+*snap_node = cur_node;
+return;
+}
+}
+if (cur_node-children)
+vboxSnapshotXmlRetrieveSnapshotNodeByName(cur_node-children,
name, snap_node);
+}
+}
+
+
+
+
+static int
+vboxDetachAndCloseDisks(virDomainPtr dom,
+IMedium *disk)
+{
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+nsresult rc;
+PRUnichar *location = NULL;
+vboxArray childrenDiskArray = VBOX_ARRAY_INITIALIZER;
+virStorageVolPtr volPtr = NULL;
+char *location_utf8 = NULL;
+PRUint32 dummyState = 0;
+size_t i = 0;
+if (disk == NULL) {
+VIR_DEBUG(Null pointer to disk);
+return -1;
+}
+rc = disk-vtbl-GetLocation(disk, location);
+if (rc == VBOX_E_ACCESSDENIED) {
+VIR_DEBUG(Disk already closed);
+goto cleanup;
+}
+reportInternalErrorIfNS_FAILED(cannot get disk location);
+rc = vboxArrayGet(childrenDiskArray, disk, disk-vtbl-GetChildren);
+reportInternalErrorIfNS_FAILED(cannot get children disks);
+for (i = 0; i childrenDiskArray.count; ++i) {
+IMedium *childDisk = childrenDiskArray.items[i];
+if (childDisk) {
+vboxDetachAndCloseDisks(dom, childDisk);
+}
+}
+rc = disk-vtbl-RefreshState(disk, dummyState);
+reportInternalErrorIfNS_FAILED(cannot refresh state);
+VBOX_UTF16_TO_UTF8(location, location_utf8);
+volPtr = vboxStorageVolLookupByPath(dom-conn, location_utf8);
+
+if (volPtr) {
+VIR_DEBUG(Closing %s, location_utf8);
+if (vboxStorageDeleteOrClose(volPtr, 0, VBOX_STORAGE_CLOSE_FLAG) != 0)
{
+VIR_DEBUG(Error while closing disk);
+}
+}
+VBOX_UTF8_FREE(location_utf8);
+cleanup:
+VBOX_UTF16_FREE(location);
+vboxArrayRelease(childrenDiskArray);
+return ret;
+}
+
+static void
+vboxSnapshotXmlAddChild(xmlNodePtr parent,
+xmlNodePtr child)
+{
+/*Used in order to add child without writing the stuff concerning xml
namespaces*/
+xmlBufferPtr tmpBuf = xmlBufferCreate();
+char *tmpString = NULL;
+xmlNodePtr tmpNode = NULL;
+xmlNodeDump(tmpBuf, parent-doc, child, 0, 0);
+ignore_value(VIR_STRDUP(tmpString, (char *)xmlBufferContent(tmpBuf)));
+xmlParseInNodeContext(parent, tmpString, (int)strlen(tmpString), 0,
tmpNode);
+if (tmpNode) {
+if (xmlAddChild(parent, xmlCopyNode(tmpNode, 1)) == NULL) {
+VIR_DEBUG(Error while adding %s to %s, (char *)tmpNode-name,
(char *)parent-name);
+}
+}
+xmlFree(tmpNode);
+xmlBufferFree(tmpBuf);
+}
+
+static void
+vboxSnapshotXmlRetrieveMachineNode(xmlNodePtr root,
+xmlNodePtr *machineNode)
+{
+xmlNodePtr cur = root-xmlChildrenNode;
+while (cur xmlIsBlankNode(cur)) {
+cur = cur - next;
+}
[libvirt] [PATCHv4 1/5] viruuid.h/c: Util method for finding uuid patterns in some strings
---
po/POTFILES.in |1 +
src/libvirt_private.syms |1 +
src/util/viruuid.c | 81 ++
src/util/viruuid.h |1 +
4 files changed, 84 insertions(+)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 15afdec..451a6fc 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -196,6 +196,7 @@ src/util/virtypedparam.c
src/util/viruri.c
src/util/virusb.c
src/util/virutil.c
+src/util/viruuid.c
src/util/virxml.c
src/vbox/vbox_MSCOMGlue.c
src/vbox/vbox_XPCOMCGlue.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a705c56..99cc32a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1915,6 +1915,7 @@ virValidateWWN;
# util/viruuid.h
virGetHostUUID;
+virSearchUuid;
virSetHostUUIDStr;
virUUIDFormat;
virUUIDGenerate;
diff --git a/src/util/viruuid.c b/src/util/viruuid.c
index c5fa9a8..2cda4ac 100644
--- a/src/util/viruuid.c
+++ b/src/util/viruuid.c
@@ -34,6 +34,7 @@
#include sys/stat.h
#include time.h
#include unistd.h
+#include regex.h
#include c-ctype.h
#include internal.h
@@ -43,11 +44,14 @@
#include viralloc.h
#include virfile.h
#include virrandom.h
+#include virstring.h
#ifndef ENODATA
# define ENODATA EIO
#endif
+#define VIR_FROM_THIS VIR_FROM_NONE
+
static unsigned char host_uuid[VIR_UUID_BUFLEN];
static int
@@ -333,3 +337,80 @@ int virGetHostUUID(unsigned char *uuid)
return ret;
}
+
+
+/**
+ * virSearchUuid:
+ * Allows you to get the nth occurrence of a substring in sourceString which
matches an uuid pattern.
+ * If there is no substring, ret is not modified.
+ * return -1 on error, 0 if not found and 1 if found.
+ *
+ * @sourceString: String to parse
+ * @occurrence: We will return the nth occurrence of uuid in substring, if
equals to 0 (or negative), will return the first occurence
+ * @ret: nth occurrence substring matching an uuid pattern
+ * @code
+char *source = 6853a496-1c10-472e-867a-8244937bd6f0
773ab075-4cd7-4fc2-8b6e-21c84e9cb391 bbb3c75c-d60f-43b0-b802-fd56b84a4222
60c04aa1-0375-4654-8d9f-e149d9885273 4548d465-9891-4c34-a184-3b1c34a26aa8;
+char *ret1=NULL;
+char *ret2=NULL;
+char *ret3=NULL;
+char *ret4=NULL;
+virSearchUuid(source, 4,ret1); //ret1 =
60c04aa1-0375-4654-8d9f-e149d9885273
+virSearchUuid(source, 0,ret2); //ret2 =
6853a496-1c10-472e-867a-8244937bd6f0
+virSearchUuid(source, 1,ret3); //ret3 =
6853a496-1c10-472e-867a-8244937bd6f0
+virSearchUuid(source, -4,ret4); //ret4 =
6853a496-1c10-472e-867a-8244937bd6f0
+ * @endcode
+ */
+
+int
+virSearchUuid(const char *sourceString, int occurrence, char **ret)
+{
+unsigned int position = ((occurrence -1) 0) ? (occurrence -1) : 0;
+const char *uuidRegex =
([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12});
+regex_t pregUuidBracket;
+size_t i = 0;
+size_t nmatch = 0;
+regmatch_t *pmatch = NULL;
+int retCode = 0;
+if (regcomp(pregUuidBracket, uuidRegex, REG_EXTENDED) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Error while compiling regular expression));
+return -1;
+}
+nmatch = pregUuidBracket.re_nsub;
+if (VIR_ALLOC_N(pmatch, nmatch) != 0) {
+retCode = -1;
+goto cleanup;
+}
+
+while (i (position+1)) {
+if (regexec(pregUuidBracket, sourceString, nmatch, pmatch, 0) == 0) {
+regoff_t start = pmatch[0].rm_so;
+regoff_t end = pmatch[0].rm_eo;
+if (i == position || (position i regexec(pregUuidBracket,
sourceString[end], nmatch, pmatch, 0) != 0)) {
+/* We copy only if i == position (so that it is the uuid we're
looking for), or position i AND
+ * there is no matches left in the rest of the string (this is
the case where we
+ * give a biggest @occurence than the number of matches and we
want to return the last
+ * one)
+ */
+if (VIR_STRNDUP(*ret, sourceString + start, end - start) 0) {
+retCode = -1;
+goto cleanup;
+}
+retCode = 1;
+goto cleanup;
+}
+
+sourceString = sourceString[end];
+} else {
+break;
+retCode = 0;
+goto cleanup;
+}
+++i;
+}
+
+cleanup:
+regfree(pregUuidBracket);
+VIR_FREE(pmatch);
+return retCode;
+}
diff --git a/src/util/viruuid.h b/src/util/viruuid.h
index bebd338..2ce4fce 100644
--- a/src/util/viruuid.h
+++ b/src/util/viruuid.h
@@ -40,4 +40,5 @@ int virUUIDParse(const char *uuidstr,
const char *virUUIDFormat(const unsigned char *uuid,
char *uuidstr) ATTRIBUTE_NONNULL(1)
ATTRIBUTE_NONNULL(2);
+int virSearchUuid(const char *sourceString, int occurrence, char **ret);
#endif /* __VIR_UUID_H__ */
--
1.7.10.4
--
libvir-list mailing list
[libvirt] [PATCHv4 5/5] vbox_tmpl.c: Add methods for undefining snapshots
All the informations concerning snapshots (and snapshot disks)
will be deleted from the vbox xml. But the differencing disks will be
kept so you will be able to redefine the snapshots.
---
src/vbox/vbox_tmpl.c | 391 ++
1 file changed, 391 insertions(+)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 23f8aab..9b50c2e 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6132,6 +6132,68 @@ vboxSnapshotXmlAppendDiskToMediaRegistry(xmlNodePtr
*inMediaRegistry,
}
}
+
+static void
+vboxRemoveAllDisksExceptParentFromMediaRegistry(xmlNodePtr mediaRegistryNode){
+xmlNodePtr cur_node = NULL;
+for (cur_node = mediaRegistryNode; cur_node; cur_node = cur_node-next) {
+if (cur_node) {
+if (cur_node-type == XML_ELEMENT_NODE
+ !xmlStrcmp(cur_node-name, (const xmlChar *) HardDisk)) {
+xmlNodePtr child = NULL;
+for (child = cur_node-children; child; child = child-next) {
+/*We look over all the children
+ *If there is a node element, we delete it
+ */
+if (child-type == XML_ELEMENT_NODE) {
+xmlUnlinkNode(child);
+xmlFreeNode(child);
+}
+}
+}
+}
+if ((cur_node-children))
+
vboxRemoveAllDisksExceptParentFromMediaRegistry((cur_node-children));
+}
+}
+
+static void
+vboxRemoveDiskFromMediaRegistryIfNoChildren(xmlNodePtr mediaRegistryNode,
+char *diskLocation)
+{
+/*
+ *This function will remove a disk from the media registry only if it
doesn't
+ *have any children
+ */
+xmlNodePtr cur_node = NULL;
+for (cur_node = mediaRegistryNode; cur_node; cur_node = cur_node-next) {
+if (cur_node) {
+if (cur_node-type == XML_ELEMENT_NODE
+ !xmlStrcmp(cur_node-name, (const xmlChar *) HardDisk)
+ xmlHasProp(cur_node, BAD_CAST location) != NULL
+ strstr(diskLocation, (char *)xmlHasProp(cur_node, BAD_CAST
location)-children-content) != NULL) {
+
+xmlNodePtr child = NULL;
+bool deleteNode = true;
+for (child = cur_node-children; child; child = child-next) {
+/*We look over all the children
+ *If there is a node element, we don't delete it
+ */
+if (child-type == XML_ELEMENT_NODE)
+deleteNode = false;
+}
+if (deleteNode) {
+xmlUnlinkNode(cur_node);
+xmlFreeNode(cur_node);
+}
+return;
+}
+}
+if ((cur_node-children))
+vboxRemoveDiskFromMediaRegistryIfNoChildren((cur_node-children),
diskLocation);
+}
+}
+
static int
vboxSnapshotGenerateVboxXML(xmlNodePtr rootElementVboxXML,
char *storageControllerString,
@@ -8076,8 +8138,319 @@ cleanup:
vboxArrayRelease(children);
return ret;
}
+#if VBOX_API_VERSION = 4002
+static int
+vboxCloseDisk(virDomainPtr dom,
+ IMedium *baseDisk) {
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+nsresult rc;
+vboxArray childrenDiskArray = VBOX_ARRAY_INITIALIZER;
+size_t i = 0;
+if (!baseDisk)
+return -1;
+
+rc = vboxArrayGet(childrenDiskArray, baseDisk,
baseDisk-vtbl-GetChildren);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(could not get children disks));
+ret = -1;
+goto cleanup;
+}
+for (i=0; i childrenDiskArray.count; ++i)
+vboxCloseDisk(dom, childrenDiskArray.items[i]);
+
+baseDisk-vtbl-Close(baseDisk);
+ret = 0;
+cleanup:
+vboxArrayRelease(childrenDiskArray);
+return ret;
+}
static int
+vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
+{
+/*This function will remove the node in the vbox xml corresponding to
+ *the snapshot. It is usually called by vboxDomainSnapshotDelete() with
+ *the flag VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY.
+ *If you want to use it anywhere else, be careful, if the snapshot you
want to delete has children,
+ *the result is not granted, they will probably will be deleted in the
xml, but you may have
+ *a problem with hard drives
+ *
+ *If the snapshot which is being deleted is the current, we will set the
current snapshot of the machine to
+ *its parent.
+ *
+ *Before the writing of the modified xml file, we undefine the machine
from vbox
+ *After the modification, we redefine the machine
+ */
+
+virDomainPtr dom = snapshot-domain;
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+IMachine *machine = NULL;
+IMachine *newMachine =
[libvirt] [PATCHv4 3/5] vbox_tmpl.c: Better XML description for snapshots
It will be needed for the futur patches because we will
redefine snapshots
---
src/conf/domain_conf.c |2 +-
src/vbox/vbox_tmpl.c | 427 ++--
2 files changed, 417 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7b0e3ea..ae20eb5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17054,7 +17054,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (virDomainChrDefFormat(buf, console, flags) 0)
goto error;
}
-if (STREQ(def-os.type, hvm)
+if (STREQ_NULLABLE(def-os.type, hvm)
def-nconsoles == 0
def-nserials 0) {
virDomainChrDef console;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 983a595..e05ed97 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -38,6 +38,7 @@
#include sys/types.h
#include sys/stat.h
#include fcntl.h
+#include libxml/xmlwriter.h
#include internal.h
#include datatypes.h
@@ -58,6 +59,8 @@
#include fdstream.h
#include viruri.h
#include virstring.h
+#include virtime.h
+#include virutil.h
/* This one changes from version to version. */
#if VBOX_API_VERSION == 2002
@@ -273,10 +276,16 @@ static vboxGlobalData *g_pVBoxGlobalData = NULL;
#endif /* VBOX_API_VERSION = 4000 */
+#define reportInternalErrorIfNS_FAILED(message) \
+if (NS_FAILED(rc)) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(message)); \
+goto cleanup; \
+}
+
+
static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml);
static int vboxDomainCreate(virDomainPtr dom);
static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags);
-
static void vboxDriverLock(vboxGlobalData *data) {
virMutexLock(data-lock);
}
@@ -285,6 +294,12 @@ static void vboxDriverUnlock(vboxGlobalData *data) {
virMutexUnlock(data-lock);
}
+typedef enum {
+VBOX_STORAGE_DELETE_FLAG = 0,
+#if VBOX_API_VERSION = 4002
+VBOX_STORAGE_CLOSE_FLAG = 1,
+#endif
+} vboxStorageDeleteOrCloseFlags;
#if VBOX_API_VERSION == 2002
static void nsIDtoChar(unsigned char *uuid, const nsID *iid) {
@@ -5957,7 +5972,8 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
if (!(def = virDomainSnapshotDefParseString(xmlDesc, data-caps,
-data-xmlopt, 0, 0)))
+data-xmlopt, -1,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE)))
goto cleanup;
if (def-ndisks) {
@@ -5965,7 +5981,6 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
_(disk snapshots not supported yet));
goto cleanup;
}
-
vboxIIDFromUUID(domiid, dom-uuid);
rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
if (NS_FAILED(rc)) {
@@ -5973,7 +5988,6 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
_(no domain with matching UUID));
goto cleanup;
}
-
rc = machine-vtbl-GetState(machine, state);
if (NS_FAILED(rc)) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
@@ -6048,6 +6062,344 @@ cleanup:
return ret;
}
+#if VBOX_API_VERSION =4002
+static
+int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def,
+virDomainSnapshotPtr snapshot)
+{
+virDomainPtr dom = snapshot-domain;
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+vboxIID domiid = VBOX_IID_INITIALIZER;
+IMachine *machine = NULL;
+ISnapshot *snap = NULL;
+IMachine *snapMachine = NULL;
+bool error = false;
+vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER;
+PRUint32 maxPortPerInst[StorageBus_Floppy + 1] = {};
+PRUint32 maxSlotPerPort[StorageBus_Floppy + 1] = {};
+int diskCount = 0;
+nsresult rc;
+vboxIID snapIid = VBOX_IID_INITIALIZER;
+char *snapshotUuidStr = NULL;
+size_t i = 0;
+vboxIIDFromUUID(domiid, dom-uuid);
+rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
+reportInternalErrorIfNS_FAILED(no domain with matching UUID);
+if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot-name))) {
+ret = -1;
+goto cleanup;
+}
+rc = snap-vtbl-GetId(snap, snapIid.value);
+reportInternalErrorIfNS_FAILED(Could not get snapshot id);
+
+VBOX_UTF16_TO_UTF8(snapIid.value, snapshotUuidStr);
+rc = snap-vtbl-GetMachine(snap, snapMachine);
+reportInternalErrorIfNS_FAILED(could not get machine);
+def-ndisks = 0;
+rc = vboxArrayGet(mediumAttachments, snapMachine,
snapMachine-vtbl-GetMediumAttachments);
+reportInternalErrorIfNS_FAILED(no medium attachments);
+/* get the number of attachments */
+for (i = 0; i mediumAttachments.count; i++) {
+IMediumAttachment *imediumattach = mediumAttachments.items[i];
+
Re: [libvirt] [PATCH libvirt-python] Update README file contents and add HACKING file
On 11/25/2013 10:18 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The previous README file from the python code is more like a HACKING file. Rename it and update the content. Then add a basic README file Signed-off-by: Daniel P. Berrange berra...@redhat.com --- HACKING | 37 + README | 41 + 2 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 HACKING diff --git a/HACKING b/HACKING + +Most of the libvirt python binding code is automatically generated +using the script generator.py, and the API description that the is 2 spaces intentional? +libvirt library installs into /usr/share (or wherever the following +command says. No closing ')'; but I'm not sure on how best to reword to fix that. Maybe: installs into the location shown by pkg-config (defaults to /usr/share) with this command: + + $ pkg-config --variable libvirt_api libvirt + /usr/share/libvirt/api/libvirt-api.xml + + +Some of the API descriptions in the primary XML files are not directlry s/directlry/directly/ +usable by the code generator. Thus there are overrides in + + - libvirt-override-api.xml + - libvirt-qemu-override-api.xml + - libvirt-lxc-override-api.xml + +For stuff which the genrator can't cope with at all there are some s/genrator/generator/ +++ b/README @@ -1,27 +1,28 @@ +The module can be built by following the normal python module +build processs s/processs/process/ +Patches for this code should be sent to the main libvirt +development mailing list + + http://libvirt.org/contact.html#email Maybe mention 'git config format.subjectprefix python PATCH' so that we get visual indication when a patch is for python. ACK with grammar cleaned up. -- 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
[libvirt] Help required in simulating libvirt TLS server
Hi All, Will some one explain how is this tls libvirt server is implemented. For my testing purpose I need to implement the similar TLS server in Java or Python and this server is capable to receive all libvirt calls like getCapabilities, hostname etc and return response as I'm configured. Basically I need to simulate the libvirt TLS server. I tried creating many TLS server but none of the my TLS server implemenation is capable to do proper handshake with python libvirt client and do successful calls. Any ideas or help will be appreciable. Thanks In Advance, Arun V -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Help required in simulating libvirt TLS server
On 11/25/2013 10:59 AM, Arun Viswanath wrote: Hi All, Will some one explain how is this tls libvirt server is implemented. For my testing purpose I need to implement the similar TLS server in Java or Python and this server is capable to receive all libvirt calls like getCapabilities, hostname etc and return response as I'm configured. Basically I need to simulate the libvirt TLS server. I tried creating many TLS server but none of the my TLS server implemenation is capable to do proper handshake with python libvirt client and do successful calls. Any ideas or help will be appreciable. I think the advice you were given last time is still accurate: https://www.redhat.com/archives/libvirt-users/2013-September/msg00086.html You are probably going to be better off hooking your simulations into the existing test:/// URI driver than trying to write your own RPC client counterparts, as the existing code will handle tls on your behalf. -- 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 01/22] conf: Implement virStorageVolType enum helper functions
On 11/25/2013 09:11 AM, Peter Krempa wrote: Add support for string conversion from and to the virStorageVolType enum. --- src/conf/storage_conf.c | 4 src/conf/storage_conf.h | 2 ++ src/libvirt_private.syms | 2 ++ 3 files changed, 8 insertions(+) You can drop most of this patch; commit 1b5c8d4 added the same. +++ b/src/libvirt_private.syms @@ -701,6 +701,8 @@ virStorageVolDefFree; virStorageVolDefParseFile; virStorageVolDefParseNode; virStorageVolDefParseString; +virStorageVolTypeFromString; +virStorageVolTypeToString; This may be the only salvageable part; if so, squash it into the patch where you first need the symbols exported. -- 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] [PATCHv4 1/8] storage: initial support for linking with libgfapi
On 11/25/2013 08:39 AM, Daniel P. Berrange wrote: On Fri, Nov 22, 2013 at 08:20:23PM -0700, Eric Blake wrote: We support gluster volumes in domain XML, so we also ought to support them as a storage pool. Besides, a future patch will want to take advantage of libgfapi to handle the case of a gluster device holding qcow2 rather than raw storage, and for that to work, we need a storage backend that can read gluster storage volume contents. This sets up the framework. ACK Pushed. We're committed now - 'gluster' storage pool will be part of 1.2.0, and I'll be trying hard to hammer on it to squash bugs during our code freeze. If there's any slight naming tweaks to XML, now's the time to decide it before it gets baked in to the release :) [I'm testing that each patch of the series still works after all the rebasing to latest, but the overall series will hopefully be upstream alongside 1/8 within the next couple hours] -- 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] [PATCHv4 2/8] storage: document gluster pool
On 11/25/2013 08:40 AM, Daniel P. Berrange wrote: On Fri, Nov 22, 2013 at 08:20:24PM -0700, Eric Blake wrote: Add support for a new pool type='gluster', similar to RBD and Sheepdog. Terminology wise, a gluster volume forms a libvirt storage pool, within the gluster volume, individual files are treated as libvirt storage volumes. * docs/schemas/storagepool.rng (poolgluster): New pool type. * docs/formatstorage.html.in: Document gluster. * docs/storage.html.in: Likewise, and contrast it with netfs. * tests/storagepoolxml2xmlin/pool-gluster.xml: New test. * tests/storagepoolxml2xmlout/pool-gluster.xml: Likewise. * tests/storagepoolxml2xmltest.c (mymain): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com ACK with the s/1.1.5/1.2.0/ change DV suggested Pushed. -- 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] [PATCHv4 3/8] storage: implement rudimentary glusterfs pool refresh
On 11/25/2013 08:52 AM, Daniel P. Berrange wrote:
On Fri, Nov 22, 2013 at 08:20:25PM -0700, Eric Blake wrote:
Actually put gfapi to use, by allowing the creation of a gluster
pool. Right now, all volumes are treated as raw and directories
are skipped; further patches will allow peering into files to
allow for qcow2 files and backing chains, and reporting proper
volume allocation. This implementation was tested against Fedora
19's glusterfs 3.4.1; it might be made simpler by requiring a
higher minimum, and/or require more hacks to work with a lower
minimum.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshPool): Initial implementation.
(virStorageBackendGlusterOpen, virStorageBackendGlusterClose)
(virStorageBackendGlusterRefreshVol): New helper functions.
Signed-off-by: Eric Blake ebl...@redhat.com
ACK
Squashed this in and pushed (in reviewing 2/8, I noticed that I had
documented that key should NOT start with a slash, but just the volume
name).
diff --git i/src/storage/storage_backend_gluster.c
w/src/storage/storage_backend_gluster.c
index 120cf0f..d57427e 100644
--- i/src/storage/storage_backend_gluster.c
+++ w/src/storage/storage_backend_gluster.c
@@ -41,8 +41,8 @@ struct _virStorageBackendGlusterState {
*
gluster[+transport]://[server[:port]]/vol/[dir/]image[?socket=...] */
virURI *uri;
-char *volname; /* vol from URI */
-char *dir; /* dir from URI, or /; always ends in '/' */
+char *volname; /* vol from URI, no '/' */
+char *dir; /* dir from URI, or /; always starts and ends in '/' */
};
typedef struct _virStorageBackendGlusterState
virStorageBackendGlusterState;
@@ -86,7 +86,7 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
if (dir) {
if (*dir != '/') {
virReportError(VIR_ERR_XML_ERROR,
- _(gluster pool path '%s' must start with /),
+ _(gluster pool path '%s' must start with /),
dir);
return NULL;
}
@@ -173,7 +173,7 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
goto cleanup;
if (VIR_STRDUP(vol-name, name) 0)
goto cleanup;
-if (virAsprintf(vol-key, /%s%s%s, state-volname, state-dir,
+if (virAsprintf(vol-key, %s%s%s, state-volname, state-dir,
vol-name) 0)
goto cleanup;
--
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
[libvirt] [PATCH RFC 1/6] qemu: snapshot: Touch up error message
---
src/qemu/qemu_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2dbaf5..96bc87e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11858,8 +11858,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
virDomainSnapshotDefPtr def,
* offline snapshots */
if (found_internal external) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(mixing internal and external snapshots is not
- supported yet));
+ _(mixing internal and external targets for a snapshot
+ is not yet supported));
goto cleanup;
}
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 0/6] Add support for snapshots on gluster.
This series has to be applied on top of the refactoring series sent earlier today. First 3 patches are additional fixes that should be good to be commited. The rest is work in progress state to gather possible comments. Peter Krempa (6): qemu: snapshot: Touch up error message qemu: snapshot: Add functions similar to disk source pool translation qemu: snapshots: Declare supported and unsupported snapshot configs RFC: snapshot: Add support for specifying snapshot disk backing type RFC: conf: snapshot: Parse more snapshot information RFC: qemu: snapshot: Add support for external active snapshots on gluster src/conf/snapshot_conf.c | 21 ++- src/conf/snapshot_conf.h | 15 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 + src/qemu/qemu_conf.c | 23 +++ src/qemu/qemu_conf.h | 6 + src/qemu/qemu_driver.c | 426 --- 7 files changed, 434 insertions(+), 68 deletions(-) -- 1.8.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 5/6] RFC: conf: snapshot: Parse more snapshot information
Add fields to hold information about network volumes for snapshots.
RFC: Missing schema and tests.
---
src/conf/snapshot_conf.c | 12
src/conf/snapshot_conf.h | 15 +--
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f6f170e..7ad605f 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -153,9 +153,9 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
if (virDomainDiskSourceDefParse(cur,
backingtype,
def-file,
-NULL,
-NULL,
-NULL,
+def-protocol,
+def-nhosts,
+def-hosts,
NULL) 0)
goto cleanup;
@@ -632,7 +632,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virDomainDiskSourceDefFormatInternal(buf,
type,
disk-file,
- 0, 0, 0, NULL, 0, NULL, NULL, 0);
+ 0,
+ disk-protocol,
+ disk-nhosts,
+ disk-hosts,
+ 0, NULL, NULL, 0);
virBufferAddLit(buf, /disk\n);
}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 241d63c..bcd92dc 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -48,12 +48,15 @@ enum virDomainSnapshotState {
typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr;
struct _virDomainSnapshotDiskDef {
-char *name; /* name matching the target dev='...' of the domain */
-int index; /* index within snapshot-dom-disks that matches name */
-int snapshot; /* enum virDomainSnapshotLocation */
-int type; /* enum virDomainDiskType */
-char *file; /* new source file when snapshot is external */
-int format; /* enum virStorageFileFormat */
+char *name; /* name matching the target dev='...' of the domain */
+int index; /* index within snapshot-dom-disks that matches name */
+int snapshot; /* enum virDomainSnapshotLocation */
+int type; /* enum virDomainDiskType */
+char *file; /* new source file when snapshot is external */
+int format; /* enum virStorageFileFormat */
+int protocol; /* network source protocol */
+size_t nhosts; /* network source hosts count */
+virDomainDiskHostDefPtr hosts; /* network source hosts */
};
/* Stores the complete snapshot metadata */
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 2/6] qemu: snapshot: Add functions similar to disk source pool translation
To avoid future pain, add placeholder functions to get the actual
snapshot disk type.
---
src/qemu/qemu_conf.c | 23 +++
src/qemu/qemu_conf.h | 6 ++
2 files changed, 29 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 1192d23..4ccca1f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1451,3 +1451,26 @@ cleanup:
virStoragePoolDefFree(pooldef);
return ret;
}
+
+
+int
+qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def)
+{
+if (def-type == -1)
+return VIR_DOMAIN_DISK_TYPE_FILE;
+
+return def-type;
+}
+
+
+int
+qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainSnapshotDiskDefPtr def)
+{
+if (def-type != VIR_DOMAIN_DISK_TYPE_VOLUME)
+return 0;
+
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Snapshots are not yet supported with 'pool' volumes));
+return -1;
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index b9786b1..0cb7901 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -29,6 +29,7 @@
# include capabilities.h
# include network_conf.h
# include domain_conf.h
+# include snapshot_conf.h
# include domain_event.h
# include virthread.h
# include security/security_manager.h
@@ -309,4 +310,9 @@ int qemuDiskGetActualType(virDomainDiskDefPtr def);
int qemuTranslateDiskSourcePool(virConnectPtr conn,
virDomainDiskDefPtr def);
+int qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def);
+
+int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
+virDomainSnapshotDiskDefPtr def);
+
#endif /* __QEMUD_CONF_H */
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 3/6] qemu: snapshots: Declare supported and unsupported snapshot configs
Currently the snapshot code did not check if it actually supports
snapshots on various disk backends for domains. To avoid future problems
add checkers that whitelist the supported configurations.
---
src/qemu/qemu_driver.c | 264 +++--
1 file changed, 234 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 96bc87e..b9c270b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11738,13 +11738,226 @@ endjob:
}
static int
-qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
+qemuDomainSnapshotPrepareDiskExternalBacking(virDomainDiskDefPtr disk)
+{
+int actualType = qemuDiskGetActualType(disk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+return 0;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+switch ((enum virDomainDiskProtocol) disk-protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+case VIR_DOMAIN_DISK_PROTOCOL_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(external inactive snapshots are not supported on
+ 'network' disks using '%s' protocol),
+ virDomainDiskProtocolTypeToString(disk-protocol));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_DISK_TYPE_DIR:
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(external inactive snapshots are not supported on
+ '%s' disks), virDomainDiskTypeToString(actualType));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr
disk)
+{
+int actualType = qemuSnapshotDiskGetActualType(disk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+return 0;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+case VIR_DOMAIN_DISK_TYPE_DIR:
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(external active snapshots are not supported on
+ '%s' disks), virDomainDiskTypeToString(actualType));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr
disk)
+{
+int actualType = qemuSnapshotDiskGetActualType(disk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+return 0;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+case VIR_DOMAIN_DISK_TYPE_DIR:
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(external inactive snapshots are not supported on
+ '%s' disks), virDomainDiskTypeToString(actualType));
+return -1;
+}
+
+return 0;
+}
+
+
+
+static int
+qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virDomainSnapshotDiskDefPtr snapdisk,
+ bool active,
+ bool reuse)
+{
+int actualType;
+struct stat st;
+
+if (qemuTranslateSnapshotDiskSourcePool(conn, snapdisk) 0)
+return -1;
+
+if (!active) {
+if (qemuTranslateDiskSourcePool(conn, disk) 0)
+return -1;
+
+if (qemuDomainSnapshotPrepareDiskExternalBacking(disk) 0)
+return -1;
+
+if (qemuDomainSnapshotPrepareDiskExternalOverlayInactive(snapdisk) 0)
+return -1;
+} else {
+if (qemuDomainSnapshotPrepareDiskExternalOverlayActive(snapdisk) 0)
+return -1;
+}
+
+actualType = qemuSnapshotDiskGetActualType(snapdisk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+if (stat(snapdisk-file, st) 0) {
+if (errno != ENOENT) {
+virReportSystemError(errno,
+ _(unable to stat for disk %s: %s),
+ snapdisk-name, snapdisk-file);
+return -1;
+}
[libvirt] [PATCH RFC 4/6] RFC: snapshot: Add support for specifying snapshot disk backing type
Add support for specifying various types when doing snapshots. This will
later allow to do snapshots on network backed volumes.
RFC: This patch is lacking tests and domain schema touch up.
---
src/conf/snapshot_conf.c | 9
src/qemu/qemu_driver.c | 56 +++-
2 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 5958f13..f6f170e 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -108,6 +108,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
{
int ret = -1;
char *snapshot = NULL;
+char *type = NULL;
xmlNodePtr cur;
def-name = virXMLPropString(node, name);
@@ -129,6 +130,13 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
}
def-type = -1;
+if ((type = virXMLPropString(node, type))) {
+if ((def-type = virDomainDiskTypeFromString(type)) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(unknown disk snapshot type '%s'), type);
+goto cleanup;
+}
+}
for (cur = node-children; cur; cur = cur-next) {
if (cur-type != XML_ELEMENT_NODE)
@@ -174,6 +182,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
ret = 0;
cleanup:
VIR_FREE(snapshot);
+VIR_FREE(type);
if (ret 0)
virDomainSnapshotDiskDefClear(def);
return ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b9c270b..e6d4f47 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12115,33 +12115,48 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
}
if (virAsprintf(device, drive-%s, disk-info.alias) 0 ||
-VIR_STRDUP(source, snap-file) 0 ||
(persistDisk VIR_STRDUP(persistSource, source) 0))
goto cleanup;
-/* create the stub file and set selinux labels; manipulate disk in
- * place, in a way that can be reverted on failure. */
-if (!reuse) {
-fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
- need_unlink, NULL);
-if (fd 0)
-goto cleanup;
-VIR_FORCE_CLOSE(fd);
-}
-
/* XXX Here, we know we are about to alter disk-backingChain if
- * successful, so we nuke the existing chain so that future
- * commands will recompute it. Better would be storing the chain
- * ourselves rather than reprobing, but this requires modifying
- * domain_conf and our XML to fully track the chain across
- * libvirtd restarts. */
+ * successful, so we nuke the existing chain so that future commands will
+ * recompute it. Better would be storing the chain ourselves rather than
+ * reprobing, but this requires modifying domain_conf and our XML to fully
+ * track the chain across libvirtd restarts. */
virStorageFileFreeMetadata(disk-backingChain);
disk-backingChain = NULL;
-if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
- VIR_DISK_CHAIN_READ_WRITE) 0) {
-qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
- VIR_DISK_CHAIN_NO_ACCESS);
+switch (snap-type) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+reuse = true;
+/* fallthrough */
+case -1: /* type was not provided in snapshot conf */
+case VIR_DOMAIN_DISK_TYPE_FILE:
+if (VIR_STRDUP(source, snap-file) 0)
+goto cleanup;
+
+/* create the stub file and set selinux labels; manipulate disk in
+ * place, in a way that can be reverted on failure. */
+if (!reuse) {
+fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
+ need_unlink, NULL);
+if (fd 0)
+goto cleanup;
+VIR_FORCE_CLOSE(fd);
+}
+
+if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+ VIR_DISK_CHAIN_READ_WRITE) 0) {
+qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+ VIR_DISK_CHAIN_NO_ACCESS);
+goto cleanup;
+}
+break;
+
+default:
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _(snapshots are not supported on '%s' volumes),
+ virDomainDiskTypeToString(snap-type));
goto cleanup;
}
@@ -12160,6 +12175,7 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
disk-src = source;
source = NULL;
disk-format = format;
+disk-type = snap-type;
if (persistDisk) {
VIR_FREE(persistDisk-src);
persistDisk-src = persistSource;
--
1.8.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 6/6] RFC: qemu: snapshot: Add support for external active snapshots on gluster
This is a basic implementation of snapshots on gluster. Many things are
lacking, especially cleanup after a failed snapshot.
---
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_command.h | 9
src/qemu/qemu_driver.c | 106
3 files changed, 109 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7a6e322..200ba45 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3826,7 +3826,7 @@ cleanup:
}
-static int
+int
qemuGetDriveSourceString(int type,
const char *src,
int protocol,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 66c23cc..ec944ea 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -312,4 +312,13 @@ qemuParseKeywords(const char *str,
int *retnkeywords,
int allowEmptyValue);
+int qemuGetDriveSourceString(int type,
+ const char *src,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ const char *username,
+ const char *secret,
+ char **path);
+
#endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e6d4f47..4aa88c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11412,6 +11412,24 @@ cleanup:
return ret;
}
+
+static int
+qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk,
+ char **source)
+{
+*source = NULL;
+
+return qemuGetDriveSourceString(qemuSnapshotDiskGetActualType(disk),
+disk-file,
+disk-protocol,
+disk-nhosts,
+disk-hosts,
+NULL,
+NULL,
+source);
+}
+
+
typedef enum {
VIR_DISK_CHAIN_NO_ACCESS,
VIR_DISK_CHAIN_READ_ONLY,
@@ -11792,6 +11810,29 @@
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
return 0;
case VIR_DOMAIN_DISK_TYPE_NETWORK:
+switch ((enum virDomainDiskProtocol) disk-protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+return 0;
+
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+case VIR_DOMAIN_DISK_PROTOCOL_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(external active snapshots are not supported on
+ 'network' disks using '%s' protocol),
+ virDomainDiskProtocolTypeToString(disk-protocol));
+return -1;
+
+}
+break;
+
case VIR_DOMAIN_DISK_TYPE_DIR:
case VIR_DOMAIN_DISK_TYPE_VOLUME:
case VIR_DOMAIN_DISK_TYPE_LAST:
@@ -12101,6 +12142,9 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm-privateData;
char *device = NULL;
char *source = NULL;
+char *newsource = NULL;
+virDomainDiskHostDefPtr newhosts = NULL;
+virDomainDiskHostDefPtr persistHosts = NULL;
int format = snap-format;
const char *formatStr = NULL;
char *persistSource = NULL;
@@ -12114,8 +12158,7 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
return -1;
}
-if (virAsprintf(device, drive-%s, disk-info.alias) 0 ||
-(persistDisk VIR_STRDUP(persistSource, source) 0))
+if (virAsprintf(device, drive-%s, disk-info.alias) 0)
goto cleanup;
/* XXX Here, we know we are about to alter disk-backingChain if
@@ -12126,14 +12169,22 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
virStorageFileFreeMetadata(disk-backingChain);
disk-backingChain = NULL;
+if (qemuDomainSnapshotDiskGetSourceString(snap, source) 0)
+goto cleanup;
+
+if (VIR_STRDUP(newsource, snap-file) 0)
+goto cleanup;
+
+if (persistDisk
+VIR_STRDUP(persistSource, snap-file) 0)
+goto cleanup;
+
switch (snap-type) {
case VIR_DOMAIN_DISK_TYPE_BLOCK:
reuse = true;
/* fallthrough */
case -1: /* type was not provided in snapshot conf */
case VIR_DOMAIN_DISK_TYPE_FILE:
-if (VIR_STRDUP(source, snap-file) 0)
-goto cleanup;
/* create the stub file and
Re: [libvirt] [PATCHv4 4/8] storage: add network-dir as new storage volume type
On 11/25/2013 08:54 AM, Daniel P. Berrange wrote:
VIR_ENUM_IMPL(virStorageVol,
VIR_STORAGE_VOL_LAST,
- file, block, dir, network)
+ file, block, dir, network, network-dir)
I've got to say I really don't like this naming but not
got a better suggestion yet.
Could we at least shorten it to 'netdir' ?
Done, and pushed (we have until 1.2.0 is actually released to change our
minds with a baked-in name, but the shorter netdir works for me).
diff --git i/docs/formatstorage.html.in w/docs/formatstorage.html.in
index d9ceeae..a089a31 100644
--- i/docs/formatstorage.html.in
+++ w/docs/formatstorage.html.in
@@ -291,7 +291,7 @@
A storage volume will generally be either a file or a device
node; span class=sincesince 1.2.0/span, an optional
output-only attribute codetype/code lists the actual type
- (file, block, dir, network, or network-dir), which is also available
+ (file, block, dir, network, or netdir), which is also available
from codevirStorageVolGetInfo()/code. The storage volume
XML format is available span class=sincesince 0.4.1/span
/p
diff --git i/docs/schemas/storagevol.rng w/docs/schemas/storagevol.rng
index 45207b4..8f07d8f 100644
--- i/docs/schemas/storagevol.rng
+++ w/docs/schemas/storagevol.rng
@@ -20,7 +20,7 @@
valueblock/value
valuedir/value
valuenetwork/value
-valuenetwork-dir/value
+valuenetdir/value
/choice
/attribute
/optional
diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index 5e8cba6..5aad75c 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -2951,8 +2951,8 @@ typedef enum {
VIR_STORAGE_VOL_BLOCK = 1,/* Block based volumes */
VIR_STORAGE_VOL_DIR = 2, /* Directory-passthrough based volume */
VIR_STORAGE_VOL_NETWORK = 3, /* Network volumes like RBD (RADOS
Block Device) */
-VIR_STORAGE_VOL_NETWORK_DIR = 4, /* Network accessible directory
that can
- * contain other network volumes */
+VIR_STORAGE_VOL_NETDIR = 4, /* Network accessible directory that can
+ * contain other network volumes */
#ifdef VIR_ENUM_SENTINELS
VIR_STORAGE_VOL_LAST
diff --git i/src/conf/storage_conf.c w/src/conf/storage_conf.c
index 3ea5bd7..22e38c1 100644
--- i/src/conf/storage_conf.c
+++ w/src/conf/storage_conf.c
@@ -53,7 +53,7 @@
VIR_ENUM_IMPL(virStorageVol,
VIR_STORAGE_VOL_LAST,
- file, block, dir, network, network-dir)
+ file, block, dir, network, netdir)
VIR_ENUM_IMPL(virStoragePool,
VIR_STORAGE_POOL_LAST,
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 6d22bc8..763417f 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -3825,7 +3825,7 @@ qemuBuildVolumeString(virConnectPtr conn,
}
break;
case VIR_STORAGE_VOL_NETWORK:
-case VIR_STORAGE_VOL_NETWORK_DIR:
+case VIR_STORAGE_VOL_NETDIR:
case VIR_STORAGE_VOL_LAST:
/* Keep the compiler quiet, qemuTranslateDiskSourcePool already
* reported the unsupported error.
diff --git i/src/qemu/qemu_conf.c w/src/qemu/qemu_conf.c
index 3c1e248..77df370 100644
--- i/src/qemu/qemu_conf.c
+++ w/src/qemu/qemu_conf.c
@@ -1377,7 +1377,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
break;
case VIR_STORAGE_VOL_NETWORK:
-case VIR_STORAGE_VOL_NETWORK_DIR:
+case VIR_STORAGE_VOL_NETDIR:
case VIR_STORAGE_VOL_LAST:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Using network volume as disk source is not
supported));
diff --git i/src/storage/storage_backend_fs.c
w/src/storage/storage_backend_fs.c
index 2af6faf..11cf2df 100644
--- i/src/storage/storage_backend_fs.c
+++ w/src/storage/storage_backend_fs.c
@@ -1159,7 +1159,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr
conn ATTRIBUTE_UNUSED,
break;
case VIR_STORAGE_VOL_BLOCK:
case VIR_STORAGE_VOL_NETWORK:
-case VIR_STORAGE_VOL_NETWORK_DIR:
+case VIR_STORAGE_VOL_NETDIR:
case VIR_STORAGE_VOL_LAST:
virReportError(VIR_ERR_NO_SUPPORT,
_(removing block or network volumes is not
supported: %s),
diff --git i/tools/virsh-volume.c w/tools/virsh-volume.c
index 66d9922..22b10d5 100644
--- i/tools/virsh-volume.c
+++ w/tools/virsh-volume.c
@@ -960,8 +960,8 @@ vshVolumeTypeToString(int type)
case VIR_STORAGE_VOL_NETWORK:
return N_(network);
-case VIR_STORAGE_VOL_NETWORK_DIR:
-return N_(net-dir);
+case VIR_STORAGE_VOL_NETDIR:
+return N_(netdir);
case VIR_STORAGE_VOL_LAST:
break;
--
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
Re: [libvirt] [PATCHv4 5/8] storage: improve directory support in gluster pool
On 11/25/2013 08:56 AM, Daniel P. Berrange wrote:
On Fri, Nov 22, 2013 at 08:20:27PM -0700, Eric Blake wrote:
diff --git a/tests/storagevolxml2xmlin/vol-gluster-dir.xml
b/tests/storagevolxml2xmlin/vol-gluster-dir.xml
new file mode 100644
index 000..bd20a6a
--- /dev/null
+++ b/tests/storagevolxml2xmlin/vol-gluster-dir.xml
@@ -0,0 +1,13 @@
+volume
+ namedir/name
+ key/vol/dir/key
+ source
+ /source
+ typenetwork-dir/type
Per my other reply I think that 'type' would be better as an attribute
on the top level volume element.
Indeed, 'make check' fails after my tweaks earlier in the series unless
I squash in this (now pushed):
diff --git i/src/storage/storage_backend_gluster.c
w/src/storage/storage_backend_gluster.c
index 51dc742..3f4e9f7 100644
--- i/src/storage/storage_backend_gluster.c
+++ w/src/storage/storage_backend_gluster.c
@@ -183,7 +183,7 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
state-uri-path = tmp;
if (S_ISDIR(st-st_mode)) {
-vol-type = VIR_STORAGE_VOL_NETWORK_DIR;
+vol-type = VIR_STORAGE_VOL_NETDIR;
vol-target.format = VIR_STORAGE_FILE_DIR;
*volptr = vol;
vol = NULL;
diff --git i/tests/storagevolxml2xmlin/vol-gluster-dir.xml
w/tests/storagevolxml2xmlin/vol-gluster-dir.xml
index bd20a6a..208c2c2 100644
--- i/tests/storagevolxml2xmlin/vol-gluster-dir.xml
+++ w/tests/storagevolxml2xmlin/vol-gluster-dir.xml
@@ -1,9 +1,8 @@
-volume
+volume type='netdir'
namedir/name
- key/vol/dir/key
+ keyvol/dir/key
source
/source
- typenetwork-dir/type
capacity unit='bytes'0/capacity
allocation unit='bytes'0/allocation
target
diff --git i/tests/storagevolxml2xmlout/vol-gluster-dir.xml
w/tests/storagevolxml2xmlout/vol-gluster-dir.xml
index 29e6d1a..f188ceb 100644
--- i/tests/storagevolxml2xmlout/vol-gluster-dir.xml
+++ w/tests/storagevolxml2xmlout/vol-gluster-dir.xml
@@ -1,9 +1,8 @@
-volume
+volume type='netdir'
namedir/name
- key/vol/dir/key
+ keyvol/dir/key
source
/source
- typenetwork-dir/type
capacity unit='bytes'0/capacity
allocation unit='bytes'0/allocation
target
--
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] [PATCHv4 6/8] storage: improve allocation stats reported on gluster files
On 11/25/2013 08:57 AM, Daniel P. Berrange wrote: On Fri, Nov 22, 2013 at 08:20:28PM -0700, Eric Blake wrote: We already had code for handling allocation different than capacity for sparse files; we just had to wire it up to be used when inspecting gluster images. * src/storage/storage_backend.c (virStorageBackendUpdateVolTargetInfoFD): Handle no fd. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Handle sparse files. Signed-off-by: Eric Blake ebl...@redhat.com --- src/storage/storage_backend.c | 8 src/storage/storage_backend_gluster.c | 7 ++- 2 files changed, 10 insertions(+), 5 deletions(-) ACK Pushed. -- 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] [PATCHv4 7/8] storage: improve handling of symlinks in gluster
On 11/25/2013 08:57 AM, Daniel P. Berrange wrote: On Fri, Nov 22, 2013 at 08:20:29PM -0700, Eric Blake wrote: With this patch, dangling and looping symlinks are silently ignored, while links to files and directories are treated the same as the underlying file or directory. This is the same behavior as both 'directory' and 'netfs' pools. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Treat symlinks similar to directory and netfs pools. Signed-off-by: Eric Blake ebl...@redhat.com --- src/storage/storage_backend_gluster.c | 11 +++ 1 file changed, 11 insertions(+) ACK Since I just added 'netdir' volume types, I'm wondering if I should also add a 'badlink' volume type (covers both ENOENT and ELOOP errors). After all, in a directory pool, you cannot create a new file with a name occupied by a bad link without first deleting the bad link; and our current behavior of silently ignoring bad links is a little unfriendly when compared with actually telling the user that they have a broken symlink, and without a way to use libvirt API to get the broken link out of the way (you can't delete something that is silently ignored). But that would be a followup patch, particularly since it affects directory (and not just gluster) pools. I've pushed this one as-is. -- 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] [PATCHv4 8/8] storage: probe qcow2 volumes in gluster pool
On 11/25/2013 08:59 AM, Daniel P. Berrange wrote:
On Fri, Nov 22, 2013 at 08:20:30PM -0700, Eric Blake wrote:
Putting together pieces from previous patches, it is now possible
for 'virsh vol-dumpxml --pool gluster volname' to report metadata
about a qcow2 file stored on gluster. The backing file is still
treated as raw; to fix that, more patches are needed to make the
storage backing chain analysis recursive rather than halting at
a network protocol name, but that work will not need any further
calls into libgfapi so much as just reusing this code, and that
should be the only code outside of the storage driver that needs
any help from libgfapi. Any additional use of libgfapi within
libvirt should only be needed for implementing storage pool APIs
such as volume creation or resizing, where backing chain analysis
should be unaffected.
+while (maxlen) {
+ssize_t r = glfs_read(fd, s, maxlen, 0);
+if (r 0 errno == EINTR)
+continue;
+if (r 0) {
+VIR_FREE(*buf);
+virReportSystemError(errno, _(unable to read '%s'), name);
+return r;
+}
Further down you're requesting O_NONBLOCK, and here you are
not handling EAGAIN explicitly. Is is desirable that we turn
EAGAIN into a fatal error, or should we remove the O_NONBLOCK
flag ?
Hmm. I was copying from directory pools, which also use O_NONBLOCK then
call into virFileReadHeaderFD. So that code needs to be fixed (separate
patch). For gluster, I think it's easiest to just drop O_NONBLOCK from
the code (I just verified that attempts to use 'mkfifo' inside a gluster
volume fail with EACCES); for directory pools you DO want to open
O_NONBLOCK (otherwise opening a fifo for read would hang waiting for a
writer), then use virSetNonBlock() after verifying file type but before
reading the header (since we already reject fifos as unable to be used
as a storage volume).
Squashing in this, then pushing.
diff --git i/src/storage/storage_backend_gluster.c
w/src/storage/storage_backend_gluster.c
index 71edb12..7e5ea9e 100644
--- i/src/storage/storage_backend_gluster.c
+++ w/src/storage/storage_backend_gluster.c
@@ -245,7 +245,9 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
vol-type = VIR_STORAGE_VOL_NETWORK;
vol-target.format = VIR_STORAGE_FILE_RAW;
-if (!(fd = glfs_open(state-vol, name, O_RDONLY| O_NONBLOCK |
O_NOCTTY))) {
+/* No need to worry about O_NONBLOCK - gluster doesn't allow creation
+ * of fifos, so there's nothing it would protect us from. */
+if (!(fd = glfs_open(state-vol, name, O_RDONLY | O_NOCTTY))) {
/* A dangling symlink now implies a TOCTTOU race; report it. */
virReportSystemError(errno, _(cannot open volume '%s'), name);
goto cleanup;
--
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
