[libvirt] [java PATCH 1/1] Add support for Qemu Guest Agent commands
Through Qemu we can send commands to a Qemu Guest Agent running inside a domain. This way we can communicate with a running Domain by asking for it's network information, requesting a filesystem trim or even execute a command inside a Domain. Commands need to be send as JSON Strings, but these have been wrapped into the QemuCommand class which has a list of pre-defined and the most commonly used commands. RAW commands can also be send for more flexibility. Signed-off-by: Wido den Hollander --- src/main/java/org/libvirt/Domain.java | 34 ++ src/main/java/org/libvirt/Library.java| 3 + .../java/org/libvirt/jna/LibvirtQemu.java | 16 +++ .../java/org/libvirt/qemu/QemuCommand.java| 106 ++ .../org/libvirt/qemu/TestQemuCommand.java | 24 5 files changed, 183 insertions(+) create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java create mode 100644 src/main/java/org/libvirt/qemu/QemuCommand.java create mode 100644 src/test/java/org/libvirt/qemu/TestQemuCommand.java diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index 83a500c..9138238 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -8,6 +8,7 @@ import org.libvirt.jna.CString; import org.libvirt.jna.DomainPointer; import org.libvirt.jna.DomainSnapshotPointer; import org.libvirt.jna.Libvirt; +import org.libvirt.jna.LibvirtQemu; import org.libvirt.jna.SizeT; import org.libvirt.jna.virDomainBlockInfo; import org.libvirt.jna.virDomainBlockStats; @@ -21,7 +22,9 @@ import org.libvirt.event.RebootListener; import org.libvirt.event.LifecycleListener; import org.libvirt.event.PMWakeupListener; import org.libvirt.event.PMSuspendListener; +import org.libvirt.qemu.QemuCommand; import static org.libvirt.Library.libvirt; +import static org.libvirt.Library.libvirtqemu; import static org.libvirt.ErrorHandler.processError; import static org.libvirt.ErrorHandler.processErrorIfZero; @@ -1556,4 +1559,35 @@ public class Domain { return processError(libvirt.virDomainUpdateDeviceFlags(VDP, xml, flags)); } +/** + * Send a Qemu Guest Agent command to a domain + * + * @param cmd + *The command which has to be send + * @param timeout + *The timeout for waiting on an answer. See QemuAgentFlags for more information. + * @param flags + *Flags to be send + * @return String + * @throws LibvirtException + */ +public String qemuAgentCommand(QemuCommand command) throws LibvirtException { +return this.qemuAgentCommand(command, 0); +} + +/** + * Send a Qemu Guest Agent command to a domain + * @see http://wiki.qemu.org/Features/QAPI/GuestAgent";>Qemu Documentation + * @param command + *The command which has to be send + * @param timeout + *The timeout for waiting on an answer. See QemuAgentFlags for more information + * @return String + * @throws LibvirtException + */ +public String qemuAgentCommand(QemuCommand command, int timeout) throws LibvirtException { +CString result = libvirtqemu.virDomainQemuAgentCommand(this.VDP, command.toString(), timeout, 0); +processError(result); +return result.toString(); +} } diff --git a/src/main/java/org/libvirt/Library.java b/src/main/java/org/libvirt/Library.java index 8e054c6..30f15be 100644 --- a/src/main/java/org/libvirt/Library.java +++ b/src/main/java/org/libvirt/Library.java @@ -2,6 +2,7 @@ package org.libvirt; import org.libvirt.jna.Libvirt; import org.libvirt.jna.Libvirt.VirEventTimeoutCallback; +import org.libvirt.jna.LibvirtQemu; import org.libvirt.jna.CString; import static org.libvirt.ErrorHandler.processError; @@ -34,6 +35,7 @@ public final class Library { }; final static Libvirt libvirt; +final static LibvirtQemu libvirtqemu; // an empty string array constant // prefer this over creating empty arrays dynamically. @@ -47,6 +49,7 @@ public final class Library { } catch (Exception e) { e.printStackTrace(); } +libvirtqemu = LibvirtQemu.INSTANCE; } private Library() {} diff --git a/src/main/java/org/libvirt/jna/LibvirtQemu.java b/src/main/java/org/libvirt/jna/LibvirtQemu.java new file mode 100644 index 000..82213e9 --- /dev/null +++ b/src/main/java/org/libvirt/jna/LibvirtQemu.java @@ -0,0 +1,16 @@ +package org.libvirt.jna; + +import com.sun.jna.Library; +import com.sun.jna.Native; +import com.sun.jna.Platform; + +/** + * The libvirt Qemu interface which is exposed via JNA + */ + +public interface LibvirtQemu extends Library { + +LibvirtQemu INSTANCE = (LibvirtQemu) Native.loadLibrary(Platform.isWindows() ? "virt-qemu-0" : "virt-qemu", LibvirtQemu.class); + +CString virDomainQemuAgentCommand(DomainPointer virDomainP
[libvirt] [java PATCH 0/1] *** Add support for sendng Qemu Guest Agent Commands ***
*** BLURB HERE *** Wido den Hollander (1): Add support for Qemu Guest Agent commands src/main/java/org/libvirt/Domain.java | 34 ++ src/main/java/org/libvirt/Library.java| 3 + .../java/org/libvirt/jna/LibvirtQemu.java | 16 +++ .../java/org/libvirt/qemu/QemuCommand.java| 106 ++ .../org/libvirt/qemu/TestQemuCommand.java | 24 5 files changed, 183 insertions(+) create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java create mode 100644 src/main/java/org/libvirt/qemu/QemuCommand.java create mode 100644 src/test/java/org/libvirt/qemu/TestQemuCommand.java -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Accepting RELATED, ESTABLISHED (TCP) connections into VM using Network Filters
Hi, Over the past few days I've been trying to get a prototype working of a stateful firewall for a Virtual Machine using Libvirt's network filters. My goal is to replace the current custom Python/Java code in the Apache CloudStack [0] project by Network Filters of Libvirt. Both IPv4 and IPv6 should work, but I started off with IPv4 and I have issues with accepting back RELATED,ESTABLISHED connections into the VM. In the guest's XML I have this defined: And the filter currently looks like this: a2493284-9dd5-4c20-98b5-7e70745b53de I can SSH into the VM and also visit the Webserver running on it. But going out the VM results in issues: root@nwfilter-test:~# telnet 109.72.92.155 80 Trying 109.72.92.155... telnet: Unable to connect to remote host: Connection refused root@nwfilter-test:~# I can however ping the same target: root@nwfilter-test:~# ping -c 2 109.72.92.155 PING 109.72.92.155 (109.72.92.155) 56(84) bytes of data. 64 bytes from 109.72.92.155: icmp_seq=1 ttl=56 time=13.2 ms 64 bytes from 109.72.92.155: icmp_seq=2 ttl=56 time=14.1 ms --- 109.72.92.155 ping statistics --- 2 packets transmitted, 2 received, 0% packet loss, time 1001ms rtt min/avg/max/mdev = 13.225/13.703/14.182/0.492 ms root@nwfilter-test:~# Looking at iptables-save it seems like the right rules are programmed: -A FI-vnet1 -p icmp -j RETURN -A FI-vnet1 -p tcp -m tcp --sport 22 -m conntrack --ctstate ESTABLISHED -m conntrack --ctdir REPLY -j RETURN -A FI-vnet1 -p tcp -m tcp --sport 80 -m conntrack --ctstate ESTABLISHED -m conntrack --ctdir REPLY -j RETURN -A FI-vnet1 -j REJECT --reject-with icmp-port-unreachable -A FO-vnet1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT -A FO-vnet1 -p icmp -j RETURN -A FO-vnet1 -p tcp -m tcp --dport 22 -m conntrack --ctstate NEW,ESTABLISHED -m conntrack --ctdir ORIGINAL -j ACCEPT -A FO-vnet1 -p tcp -m tcp --dport 80 -m conntrack --ctstate NEW,ESTABLISHED -m conntrack --ctdir ORIGINAL -j ACCEPT -A FO-vnet1 -j REJECT --reject-with icmp-port-unreachable -A HI-vnet1 -p icmp -j RETURN -A HI-vnet1 -p tcp -m tcp --sport 22 -m conntrack --ctstate ESTABLISHED -m conntrack --ctdir REPLY -j RETURN -A HI-vnet1 -p tcp -m tcp --sport 80 -m conntrack --ctstate ESTABLISHED -m conntrack --ctdir REPLY -j RETURN -A HI-vnet1 -j REJECT --reject-with icmp-port-unreachable -A libvirt-host-in -m physdev --physdev-in vnet1 -g HI-vnet1 -A libvirt-in -m physdev --physdev-in vnet1 -g FI-vnet1 -A libvirt-in-post -m physdev --physdev-in vnet1 -j ACCEPT -A libvirt-out -m physdev --physdev-out vnet1 --physdev-is-bridged -g FO-vnet1 I tried changing 'accept' into 'return' for the incoming RELATED,ESTABLISHED rules, but that didn't help. I also tried searching for example of more complex network filters, but all I keep finding are the default filters of Libvirt. Does anybody know what I'm doing wrong here? Or are there any examples of working filters out there? Thank you! Wido [0]: http://cloudstack.apache.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] is there a plan to support internal-rbd-snapshot?
> Op 23 juni 2016 om 1:50 schreef John Ferlan : > > > > > On 06/20/2016 10:35 PM, longguang.yue wrote: > > hi, is there a plan to support internal-rbd-snapshot? > > any one is responsible for it? > > > > thanks > > > > Typically patches come from Wido den Hollander whom I've CC'd so > hopefully he can answer... > > Perhaps it has something to do with something submitted over the recent > December holiday that got no attention :-(: > > http://www.redhat.com/archives/libvir-list/2015-December/msg00892.html Yes, it was my idea to support the snapshots of RBD in libvirt, but since the storage pool driver had no idea/concept of snapshots for volumes I gave it a try in that patch. If the storage driver would get support for volume snapshots I'd be happy to implement this into the RBD driver. Wido > > John > > > > > > > > > > > > -- > > 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 3/3] rbd: Use RBD fast-diff for querying actual volume allocation
Since Ceph version Infernalis (9.2.0) the new fast-diff mechanism of RBD allows for querying actual volume usage. Prior to this version there was no easy and fast way to query how much allocation a RBD volume had inside a Ceph cluster. To use the fast-diff feature it needs to be enabled per RBD image and is only supported by Ceph cluster running version Infernalis (9.2.0) or newer. Without the fast-diff feature enabled libvirt will report an allocation identical to the image capacity. This is how libvirt behaves currently. 'virsh vol-info rbd/image2' might output for example: Name: image2 Type: network Capacity: 1,00 GiB Allocation: 124,00 MiB Newly created volumes will have the fast-diff feature enabled if the backing Ceph cluster supports it. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 84 +-- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 887db0b..c00f6b2 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -297,6 +297,68 @@ volStorageBackendRBDGetFeatures(rbd_image_t image, return ret; } +#if LIBRBD_VERSION_CODE > 265 +static bool +volStorageBackendRBDUseFastDiff(uint64_t features) +{ +return features & RBD_FEATURE_FAST_DIFF; +} + +static int +virStorageBackendRBDRefreshVolInfoCb(uint64_t offset ATTRIBUTE_UNUSED, + size_t len, + int exists, + void *arg) +{ +uint64_t *used_size = (uint64_t *)(arg); +if (exists) +(*used_size) += len; + +return 0; +} + +static int +virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol, + rbd_image_t *image, + rbd_image_info_t *info) +{ +int r, ret = -1; +uint64_t allocation = 0; + +if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1, + &virStorageBackendRBDRefreshVolInfoCb, + &allocation)) < 0) { +virReportSystemError(-r, _("failed to iterate RBD image '%s'"), + vol->name); +goto cleanup; +} + +VIR_DEBUG("Found %zu bytes allocated for RBD image %s", + allocation, vol->name); + +vol->target.allocation = allocation; +ret = 0; + + cleanup: +return ret; +} + +#else +static int +volStorageBackendRBDUseFastDiff(uint64_t features ATTRIBUTE_UNUSED) +{ +return false; +} + +static int +virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol ATTRIBUTE_UNUSED, + rbd_image_t *image ATTRIBUTE_UNUSED, + rbd_image_info_t *info ATTRIBUTE_UNUSED) +{ +return false; +} +#endif + static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStoragePoolObjPtr pool, @@ -306,6 +368,7 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, int r = 0; rbd_image_t image = NULL; rbd_image_info_t info; +uint64_t features; if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { ret = -r; @@ -321,15 +384,30 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, goto cleanup; } -VIR_DEBUG("Refreshed RBD image %s/%s (size: %zu obj_size: %zu num_objs: %zu)", - pool->def->source.name, vol->name, info.size, info.obj_size, - info.num_objs); +if (volStorageBackendRBDGetFeatures(image, vol->name, &features) < 0) +goto cleanup; vol->target.capacity = info.size; vol->target.allocation = info.obj_size * info.num_objs; vol->type = VIR_STORAGE_VOL_NETWORK; vol->target.format = VIR_STORAGE_FILE_RAW; +if (volStorageBackendRBDUseFastDiff(features)) { +VIR_DEBUG("RBD image %s/%s has fast-diff feature enabled. " + "Querying for actual allocation", + pool->def->source.name, vol->name); + +if (virStorageBackendRBDSetAllocation(vol, image, &info) < 0) +goto cleanup; +} else { +vol->target.allocation = info.obj_size * info.num_objs; +} + +VIR_DEBUG("Refreshed RBD image %s/%s (capacity: %llu allocation: %llu " + "obj_size: %zu num_objs: %zu)", + pool->def->source.name, vol->name, vol->target.capacity, + vol->target.allocation, info.obj_size, info.num_objs); + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->source.name, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] rbd: Add volStorageBackendRBDGetFeatures() for internal calls
As more and more features are added to RBD volumes we will need to call this method more often. By moving it into a internal function we can re-use code inside the storage backend. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5d73370..5f2469f 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -280,6 +280,24 @@ virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) } static int +volStorageBackendRBDGetFeatures(rbd_image_t image, +const char *volname, +uint64_t *features) +{ +int r, ret = -1; + +if ((r = rbd_get_features(image, features)) < 0) { +virReportSystemError(-r, _("failed to get the features of RBD image " + "%s"), volname); +goto cleanup; +} +ret = 0; + + cleanup: +return ret; +} + +static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStoragePoolObjPtr pool, virStorageBackendRBDStatePtr ptr) @@ -685,11 +703,8 @@ virStorageBackendRBDImageInfo(rbd_image_t image, goto cleanup; } -if ((r = rbd_get_features(image, features)) < 0) { -virReportSystemError(-r, _("failed to get the features of RBD image %s"), - volname); +if (volStorageBackendRBDGetFeatures(image, volname, features) < 0) goto cleanup; -} if ((r = rbd_get_stripe_unit(image, stripe_unit)) < 0) { virReportSystemError(-r, _("failed to get the stripe unit of RBD image %s"), -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Improvements to the RBD storage backend
Hi, After review by John Ferlan I'm sending a revised version of my earlier patch which adds the fast-diff feature for RBD volumes. Before the patch there is one small improvement which is later used by the fast-diff feature. The other is a bugfix where the #ifdef in the code wasn't requiring the right version of librbd. Wido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] rbd: rbd_diff_iterate2() is available in librbd since 266
In commit 0b15f920 there is a #ifdef which requires LIBRBD_VERSION_CODE 266 or newer for rbd_diff_iterate2() rbd_diff_iterate2() is available since 266, so this if-statement should require anything newer than 265. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5f2469f..887db0b 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -792,8 +792,10 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, * rbd_diff_iterate2() is available in versions above Ceph 0.94 (Hammer) * It uses a object map inside Ceph which is faster than rbd_diff_iterate() * which iterates all objects. + * LIBRBD_VERSION_CODE for Ceph 0.94 is 265. In 266 and upwards diff_iterate2 + * is available */ -#if LIBRBD_VERSION_CODE > 266 +#if LIBRBD_VERSION_CODE > 265 r = rbd_diff_iterate2(image, snaps[i].name, 0, info.size, 0, 1, virStorageBackendRBDIterateCb, (void *)&diff); #else -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Use RBD fast-diff for querying actual volume allocation
Since Ceph version Infernalis (9.2.0) the new fast-diff mechanism of RBD allows for querying actual volume usage. Prior to this version there was no easy and fast way to query how much allocation a RBD volume had inside a Ceph cluster. To use the fast-diff feature it needs to be enabled per RBD image and is only supported by Ceph cluster running version Infernalis (9.2.0) or newer. Without the fast-diff feature enabled libvirt will report an allocation identical to the image capacity. This is how libvirt behaves currently. 'virsh vol-info rbd/image2' might output for example: Name: image2 Type: network Capacity: 1,00 GiB Allocation: 124,00 MiB Newly created volumes will have the fast-diff feature enabled if the backing Ceph cluster supports it. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 48 +++ 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5d73370..cdcfa48 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -279,6 +279,20 @@ virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) return ret; } +#if LIBRBD_VERSION_CODE > 265 +static int +virStorageBackendRBDRefreshVolInfoCb(uint64_t offset ATTRIBUTE_UNUSED, + size_t len, + int exists, + void *arg) { +uint64_t *used_size = (uint64_t *)(arg); +if (exists) +(*used_size) += len; + +return 0; +} +#endif + static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStoragePoolObjPtr pool, @@ -288,6 +302,8 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, int r = 0; rbd_image_t image = NULL; rbd_image_info_t info; +uint64_t features ATTRIBUTE_UNUSED; +uint64_t used_size ATTRIBUTE_UNUSED = 0; if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { ret = -r; @@ -303,15 +319,39 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, goto cleanup; } -VIR_DEBUG("Refreshed RBD image %s/%s (size: %zu obj_size: %zu num_objs: %zu)", - pool->def->source.name, vol->name, info.size, info.obj_size, - info.num_objs); - vol->target.capacity = info.size; vol->target.allocation = info.obj_size * info.num_objs; vol->type = VIR_STORAGE_VOL_NETWORK; vol->target.format = VIR_STORAGE_FILE_RAW; +#if LIBRBD_VERSION_CODE > 265 +if ((r = rbd_get_features(image, &features)) < 0) { +virReportSystemError(-r, _("failed to get flags of RBD image '%s'"), + vol->name); +goto cleanup; +} + +if (features & RBD_FEATURE_FAST_DIFF) { +VIR_DEBUG("RBD image %s/%s has fast-diff enabled. Querying allocation", + pool->def->source.name, vol->name); + +if ((r = rbd_diff_iterate2(image, NULL, 0, info.size, 0, 1, + &virStorageBackendRBDRefreshVolInfoCb, + &used_size)) < 0) { +virReportSystemError(-r, _("failed to iterate RBD image '%s'"), + vol->name); +goto cleanup; +} + +vol->target.allocation = used_size; +} +#endif + +VIR_DEBUG("Refreshed RBD image %s/%s (capacity: %llu allocation: %llu " + "obj_size: %zu num_objs: %zu)", + pool->def->source.name, vol->name, vol->target.capacity, + vol->target.allocation, info.obj_size, info.num_objs); + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->source.name, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virStream: this function is not supported by the connection driver
> Op 1 februari 2016 om 14:11 schreef Michal Privoznik : > > > On 31.01.2016 12:03, Wido den Hollander wrote: > > > > > > On 01/31/2016 08:37 AM, Michal Privoznik wrote: > >> On 30.01.2016 19:59, Wido den Hollander wrote: > >>> Hi, > >>> > >>> I'm trying to implement the volUpload and volDownload functions for the > >>> RBD storage driver but I keep getting this: > >>> > >>> 2016-01-30 18:56:11.675+: 6447: debug : > >>> virStorageBackendRBDVolDownload:1395 : Read 4096 bytes at offset 0 from > >>> RBD image libvirt/wido1 > >>> > >>> 2016-01-30 18:56:11.675+: 6447: debug : virStreamSend:168 : > >>> stream=0x7fe78930, data=0x7fe780062df0, nbytes=4096 > >>> > >>> 2016-01-30 18:56:11.675+: 6447: error : virStreamSend:186 : this > >>> function is not supported by the connection driver: virStreamSend > >>> > >>> I reference to the stream with virStreamRef(stream); and then I use > >>> virStreamSend() to write data to the stream, but that fails. > >>> > >>> I've looked through the source and couldn't find anything what might be > >>> the issue. > >>> > >>> Libvirt is running locally on my system and I'm using qemu:///system to > >>> connect locally with virsh. > >>> > >>> Do I need to initialize the stream in any way before I can write data to > >>> it? > >> > >> Interesting, seems like all virStream drivers have implemented Send() > >> and Recv() (what's the point in having them if they haven't, right?). > >> How are you creating the stream? You know that you need to create the > >> stream on both client and server side, right? > >> > > > > It's in the storage backend driver, so I assume the stream has already > > been created. > > > > This is the code: > > https://github.com/wido/libvirt/commit/bbb6403ebd952d7c3f2fba8c60f77087e06a2a22 > > Oh, slightly unrelated: you should move @buf allocation out of while() - > you need to allocate it only once, not each time per iteration. > Thanks, I'll take a look at that as well. > > > > static int > > virStorageBackendRBDVolDownload(virConnectPtr conn, > > virStoragePoolObjPtr pool, > > virStorageVolDefPtr vol, > > virStreamPtr stream, > > unsigned long long offset, > > unsigned long long length, > > unsigned int flags) > > .. > > .. > > virStreamRef(stream); > > .. > > .. > > while (1) { > > rbd_read() > > virStreamSend(X, stream) > > } > > > > > > And that gives me the error I posted above. > > > > I checked the docs: https://libvirt.org/html/libvirt-libvirt-stream.html > > > > Couldn't find anything additional about what I needed to do. I assume > > the storage driver creates the stream for me. > > Actually, it's daemon dispatch function that creates it in > remoteDispatchStorageVolDownload(). Now the question is, what connection > URI you're using and whether it is a remote one (so that the daemon is > involved). If not, you need to create the stream on your own. On the > other hand - I don't know how are you testing this - virsh does create > the stream for you. > Simple, it's my local system running libvirtd: $ env LIBVIRT_DEBUG=1 ./run ./daemon/libvirtd -f libvirtd.conf -v > libvirt.log 2>&1 Afterwards I run: $ ./tools/virsh pool-start rbdpool $ ./tools/virsh vol-download libvirt/myvolume /tmp/myvolume In that case the stream should have been created by virsh, right? > What you can do is to set breakpoint at virStreamSend and see what does > @st look like. st->driver should point to actual driver implementation > and you should see it filled with callbacks. > Good point, I'll look at that as well. > I have no RBD set up to help you more. Sorry. > Np. This doesn't seem RBD related, but mainly virStream. Wido > Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virStream: this function is not supported by the connection driver
On 01/31/2016 08:37 AM, Michal Privoznik wrote: > On 30.01.2016 19:59, Wido den Hollander wrote: >> Hi, >> >> I'm trying to implement the volUpload and volDownload functions for the >> RBD storage driver but I keep getting this: >> >> 2016-01-30 18:56:11.675+: 6447: debug : >> virStorageBackendRBDVolDownload:1395 : Read 4096 bytes at offset 0 from >> RBD image libvirt/wido1 >> >> 2016-01-30 18:56:11.675+: 6447: debug : virStreamSend:168 : >> stream=0x7fe78930, data=0x7fe780062df0, nbytes=4096 >> >> 2016-01-30 18:56:11.675+: 6447: error : virStreamSend:186 : this >> function is not supported by the connection driver: virStreamSend >> >> I reference to the stream with virStreamRef(stream); and then I use >> virStreamSend() to write data to the stream, but that fails. >> >> I've looked through the source and couldn't find anything what might be >> the issue. >> >> Libvirt is running locally on my system and I'm using qemu:///system to >> connect locally with virsh. >> >> Do I need to initialize the stream in any way before I can write data to it? > > Interesting, seems like all virStream drivers have implemented Send() > and Recv() (what's the point in having them if they haven't, right?). > How are you creating the stream? You know that you need to create the > stream on both client and server side, right? > It's in the storage backend driver, so I assume the stream has already been created. This is the code: https://github.com/wido/libvirt/commit/bbb6403ebd952d7c3f2fba8c60f77087e06a2a22 static int virStorageBackendRBDVolDownload(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, unsigned long long length, unsigned int flags) .. .. virStreamRef(stream); .. .. while (1) { rbd_read() virStreamSend(X, stream) } And that gives me the error I posted above. I checked the docs: https://libvirt.org/html/libvirt-libvirt-stream.html Couldn't find anything additional about what I needed to do. I assume the storage driver creates the stream for me. Wido > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virStream: this function is not supported by the connection driver
Hi, I'm trying to implement the volUpload and volDownload functions for the RBD storage driver but I keep getting this: 2016-01-30 18:56:11.675+: 6447: debug : virStorageBackendRBDVolDownload:1395 : Read 4096 bytes at offset 0 from RBD image libvirt/wido1 2016-01-30 18:56:11.675+: 6447: debug : virStreamSend:168 : stream=0x7fe78930, data=0x7fe780062df0, nbytes=4096 2016-01-30 18:56:11.675+: 6447: error : virStreamSend:186 : this function is not supported by the connection driver: virStreamSend I reference to the stream with virStreamRef(stream); and then I use virStreamSend() to write data to the stream, but that fails. I've looked through the source and couldn't find anything what might be the issue. Libvirt is running locally on my system and I'm using qemu:///system to connect locally with virsh. Do I need to initialize the stream in any way before I can write data to it? Wido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] rbd: Use %zu for uint64_t instead of casting to unsigned long long
This was only used in debugging messages and not in any real code. Ceph/RBD uses uint64_t for sizes internally and they can be printed with %zu without any need for casting. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 4a8d90d..5d73370 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -303,10 +303,9 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, goto cleanup; } -VIR_DEBUG("Refreshed RBD image %s/%s (size: %llu obj_size: %llu num_objs: %llu)", - pool->def->source.name, vol->name, (unsigned long long)info.size, - (unsigned long long)info.obj_size, - (unsigned long long)info.num_objs); +VIR_DEBUG("Refreshed RBD image %s/%s (size: %zu obj_size: %zu num_objs: %zu)", + pool->def->source.name, vol->name, info.size, info.obj_size, + info.num_objs); vol->target.capacity = info.size; vol->target.allocation = info.obj_size * info.num_objs; @@ -369,10 +368,9 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, pool->def->available = clusterstat.kb_avail * 1024; pool->def->allocation = poolstat.num_bytes; -VIR_DEBUG("Utilization of RBD pool %s: (kb: %llu kb_avail: %llu num_bytes: %llu)", - pool->def->source.name, (unsigned long long)clusterstat.kb, - (unsigned long long)clusterstat.kb_avail, - (unsigned long long)poolstat.num_bytes); +VIR_DEBUG("Utilization of RBD pool %s: (kb: %zu kb_avail: %zu num_bytes: %zu)", + pool->def->source.name, clusterstat.kb, clusterstat.kb_avail, + poolstat.num_bytes); while (true) { if (VIR_ALLOC_N(names, max_size) < 0) -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] rbd: Code styling cleanup
Through the years the RBD storage pool code hasn't maintained the same or correct coding standard which applies to libvirt. This patch doesn't change any logic in the code, it only applies the proper coding standards to the code where possible without making large changes. This way the code style used in this storage pool is consistent throughout the whole file. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 108 +++--- 1 file changed, 53 insertions(+), 55 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80a1d33..4a8d90d 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -49,9 +49,10 @@ struct _virStorageBackendRBDState { typedef struct _virStorageBackendRBDState virStorageBackendRBDState; typedef virStorageBackendRBDState *virStorageBackendRBDStatePtr; -static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, - virConnectPtr conn, - virStoragePoolSourcePtr source) +static int +virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, + virConnectPtr conn, + virStoragePoolSourcePtr source) { int ret = -1; int r = 0; @@ -71,8 +72,8 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); -r = rados_create(&ptr->cluster, authdef->username); -if (r < 0) { + +if ((r = rados_create(&ptr->cluster, authdef->username)) < 0) { virReportSystemError(-r, "%s", _("failed to initialize RADOS")); goto cleanup; } @@ -222,8 +223,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, rados_conf_set(ptr->cluster, "rbd_default_format", rbd_default_format); ptr->starttime = time(0); -r = rados_connect(ptr->cluster); -if (r < 0) { +if ((r = rados_connect(ptr->cluster)) < 0) { virReportSystemError(-r, _("failed to connect to the RADOS monitor on: %s"), mon_buff); goto cleanup; @@ -242,7 +242,9 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, return ret; } -static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) +static int +virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, + virStoragePoolObjPtr pool) { int r = rados_ioctx_create(ptr->cluster, pool->def->source.name, &ptr->ioctx); if (r < 0) { @@ -252,7 +254,8 @@ static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virSt return r; } -static int virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) +static int +virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) { int ret = 0; @@ -276,25 +279,24 @@ static int virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) return ret; } -static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, - virStoragePoolObjPtr pool, - virStorageBackendRBDStatePtr ptr) +static int +volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, + virStoragePoolObjPtr pool, + virStorageBackendRBDStatePtr ptr) { int ret = -1; int r = 0; rbd_image_t image = NULL; +rbd_image_info_t info; -r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL); -if (r < 0) { +if ((r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL)) < 0) { ret = -r; virReportSystemError(-r, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; } -rbd_image_info_t info; -r = rbd_stat(image, &info, sizeof(info)); -if (r < 0) { +if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { ret = -r; virReportSystemError(-r, _("failed to stat the RBD image '%s'"), vol->name); @@ -331,8 +333,9 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, return ret; } -static int virStorageBackendRBDRefreshPool(virConnectPtr conn, - virStoragePoolObjPtr pool) +static int +virStorageBackendRBDRefreshPool(virConnectPtr conn, +virStoragePoolObjPtr pool) { size_t max
[libvirt] [PATCH] rbd: Allow shrinking and deltas when resizing RBD volumes
Modify virCheckFlags that it accepts both VIR_STORAGE_VOL_RESIZE_DELTA and VIR_STORAGE_VOL_RESIZE_SHRINK as valid flags for resizing RBD volumes. This still does not solve the problem where RBD volumes can't be shrinked using libvirt. The allocation and capacity of RBD volumes are equal for a RBD volume and this makes libvirt think the volume is fully allocated and therefor can't be shrinked. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80a1d33..88b613a 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1041,7 +1041,8 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, int ret = -1; int r = 0; -virCheckFlags(0, -1); +virCheckFlags(VIR_STORAGE_VOL_RESIZE_DELTA | + VIR_STORAGE_VOL_RESIZE_SHRINK, -1); if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0) goto cleanup; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Open in Read-Only mode when refreshing a volume
By opening a RBD volume in Read-Only we do not register a watcher on the header object inside the Ceph cluster. Refreshing a volume only calls rbd_stat() which is a operation which does not write to a RBD image. This allows us to use a cephx user which has no write permissions if we would want to use the libvirt storage pool for informational purposes only. It also saves us a write into the Ceph cluster which should speed up refreshing a RBD pool. rbd_open_read_only() is available in all librbd versions which also support rbd_open(). Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8c7a80d..3ab7912 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -283,7 +283,7 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, int r = 0; rbd_image_t image = NULL; -r = rbd_open(ptr->ioctx, vol->name, &image, NULL); +r = rbd_open_read_only(ptr->ioctx, vol->name, &image, NULL); if (r < 0) { ret = -r; virReportSystemError(-r, _("failed to open the RBD image '%s'"), -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] rbd: Add support for wiping RBD volumes using TRIM.
Using VIR_STORAGE_VOL_WIPE_ALG_TRIM a RBD volume can be trimmed down to 0 bytes using rbd_dicard() Effectively all the data on the volume will be lost/gone, but the volume remains available for use afterwards. Starting at offset 0 the storage pool will call rbd_discard() in stripe size * count increments which is usually 4MB. Stripe size being 4MB and count 1. rbd_discard() is available since Ceph version Dumpling (0.67) which dates back to August 2013. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 42 ++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 7e669ff..8a95388 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -772,6 +772,41 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, } static int +virStorageBackendRBDVolWipeDiscard(rbd_image_t image, + char *imgname, + rbd_image_info_t info, + uint64_t stripe_count) +{ +int r = -1; +int ret = -1; +uint64_t offset = 0; +uint64_t length; + +VIR_DEBUG("Wiping RBD %s volume using discard)", imgname); + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +if ((r = rbd_discard(image, offset, length)) < 0) { +virReportSystemError(-r, _("discarding %zu bytes failed on " + "RBD image %s at offset %zu"), + length, imgname, offset); +goto cleanup; +} + +VIR_DEBUG("Discarded %zu bytes of RBD image %s at offset %zu", + length, imgname, offset); + +offset += length; +} + +ret = 0; + + cleanup: +return ret; +} + +static int virStorageBackendRBDVolWipe(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -787,7 +822,8 @@ virStorageBackendRBDVolWipe(virConnectPtr conn, int r = -1; int ret = -1; -virCheckFlags(VIR_STORAGE_VOL_WIPE_ALG_ZERO, -1); +virCheckFlags(VIR_STORAGE_VOL_WIPE_ALG_ZERO | + VIR_STORAGE_VOL_WIPE_ALG_TRIM, -1); VIR_DEBUG("Wiping RBD image %s/%s", pool->def->source.name, vol->name); @@ -823,6 +859,10 @@ virStorageBackendRBDVolWipe(virConnectPtr conn, r = virStorageBackendRBDVolWipeZero(image, vol->name, info, stripe_count); break; +case VIR_STORAGE_VOL_WIPE_ALG_TRIM: +r = virStorageBackendRBDVolWipeDiscard(image, vol->name, + info, stripe_count); +break; default: virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"), algorithm); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Storage backend and RBD improvements
This series of patches is mainly focussed on improving the RBD storage pool backend. In the series it fixes a bug in the storage pool driver regarding volume wiping and it also adds the new flag to instruct storage pools to use TRIM for wiping volumes. Afterwards it adds this functionality to the RBD storage pool and in addition it also adds to ability to clone RBD volumes. The original series of patches were reviewed by John Ferlan and this series are revised based on his great feedback. Wido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] rbd: Implement buildVolFrom using RBD cloning
RBD supports cloning by creating a snapshot, protecting it and create a child image based on that snapshot afterwards. The RBD storage driver will try to find a snapshot with zero deltas between the current state of the original volume and the snapshot. If such a snapshot is found a clone/child image will be created using the rbd_clone2() function from librbd. rbd_clone2() is available in librbd since Ceph version Dumpling (0.67) which dates back to August 2013. It will use the same features, strip size and stripe count as the parent image. This implementation will only create a single snapshot on the parent image if never changes. This reduces the amount of snapshots created for that RBD image which benefits the performance of the Ceph cluster. During build the decision will be made to use either rbd_diff_iterate() or rbd_diff_iterate2(). The latter is faster, but only available on Ceph versions after 0.94 (Hammer). Cloning is only supported if RBD format 2 is used. All images created by libvirt are already format 2. If a RBD format 1 image is used as the original volume the backend will report a VIR_ERR_OPERATION_UNSUPPORTED error. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 341 ++ 1 file changed, 341 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8a95388..1fe6667 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -32,6 +32,7 @@ #include "base64.h" #include "viruuid.h" #include "virstring.h" +#include "virrandom.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -663,6 +664,345 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, return ret; } +static int +virStorageBackendRBDImageInfo(rbd_image_t image, + char *volname, + uint64_t *features, + uint64_t *stripe_unit, + uint64_t *stripe_count) +{ +int ret = -1; +int r = 0; +uint8_t oldformat; + +if ((r = rbd_get_old_format(image, &oldformat)) < 0) { +virReportSystemError(-r, _("failed to get the format of RBD image %s"), + volname); +goto cleanup; +} + +if (oldformat != 0) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("RBD image %s is old format. Does not support " + "extended features and striping"), + volname); +goto cleanup; +} + +if ((r = rbd_get_features(image, features)) < 0) { +virReportSystemError(-r, _("failed to get the features of RBD image %s"), + volname); +goto cleanup; +} + +if ((r = rbd_get_stripe_unit(image, stripe_unit)) < 0) { +virReportSystemError(-r, _("failed to get the stripe unit of RBD image %s"), + volname); +goto cleanup; +} + +if ((r = rbd_get_stripe_count(image, stripe_count)) < 0) { +virReportSystemError(-r, _("failed to get the stripe count of RBD image %s"), + volname); +goto cleanup; +} + +ret = 0; + + cleanup: +return ret; +} + +/* Callback function for rbd_diff_iterate() */ +static int +virStorageBackendRBDIterateCb(uint64_t offset ATTRIBUTE_UNUSED, + size_t length ATTRIBUTE_UNUSED, + int exists ATTRIBUTE_UNUSED, + void *arg) +{ +/* + * Just set that there is a diff for this snapshot, we do not care where + * + * When it returns a negative number the rbd_diff_iterate() function will stop + * + * That's why we return -1, meaning that there is a difference and we can stop + * searching any further. + */ +*(int*) arg = 1; +return -1; +} + +static int +virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, + char *imgname, + virBufferPtr snapname) +{ +int r = -1; +int ret = -1; +int snap_count; +int max_snaps = 128; +size_t i; +int diff; +rbd_snap_info_t *snaps = NULL; +rbd_image_info_t info; + +if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { +virReportSystemError(-r, _("failed to stat the RBD image %s"), + imgname); +goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) +VIR_FREE(snaps); + +} while (snap_count == -ERANGE); + +if (snap_count <= 0) { +if (snap_count == 0
[libvirt] [PATCH 1/5] storage: Properly check flags when wiping volumes
Using virCheckFlags() we validate if the provided flags are support flags by this function. The old code would also still run the command since it didn't go to cleanup when a invalid flag was supplied. We now go to cleanup and exit if a invalid flag would be provided. Signed-off-by: Wido den Hollander --- src/storage/storage_backend.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 03bc18c..15e9109 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2056,7 +2056,16 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, struct stat st; virCommandPtr cmd = NULL; -virCheckFlags(0, -1); +virCheckFlags(VIR_STORAGE_VOL_WIPE_ALG_ZERO | + VIR_STORAGE_VOL_WIPE_ALG_NNSA | + VIR_STORAGE_VOL_WIPE_ALG_DOD | + VIR_STORAGE_VOL_WIPE_ALG_BSI | + VIR_STORAGE_VOL_WIPE_ALG_GUTMANN | + VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER | + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 | + VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 | + VIR_STORAGE_VOL_WIPE_ALG_RANDOM | + VIR_STORAGE_VOL_WIPE_ALG_LAST, -1); VIR_DEBUG("Wiping volume with path '%s' and algorithm %u", vol->target.path, algorithm); @@ -2078,7 +2087,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, if (algorithm != VIR_STORAGE_VOL_WIPE_ALG_ZERO) { const char *alg_char ATTRIBUTE_UNUSED = NULL; -switch (algorithm) { +switch ((virStorageVolWipeAlgorithm) algorithm) { case VIR_STORAGE_VOL_WIPE_ALG_NNSA: alg_char = "nnsa"; break; @@ -2107,6 +2116,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"), algorithm); +goto cleanup; } cmd = virCommandNew(SCRUB); virCommandAddArgList(cmd, "-f", "-p", alg_char, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] rbd: Add support for wiping RBD volumes
When wiping the RBD image will be filled with zeros started at offset 0 and until the end of the volume. This will result in the RBD volume growing to it's full allocation on the Ceph cluster. All data on the volume will be overwritten however, making it unavailable. It does NOT take any RBD snapshots into account. The original data might still be in a snapshot of that RBD volume. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 115 ++ 1 file changed, 115 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8c7a80d..7e669ff 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -732,6 +732,120 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +static int +virStorageBackendRBDVolWipeZero(rbd_image_t image, +char *imgname, +rbd_image_info_t info, +uint64_t stripe_count) +{ +int r = -1; +int ret = -1; +uint64_t offset = 0; +uint64_t length; +char *writebuf; + +if (VIR_ALLOC_N(writebuf, info.obj_size * stripe_count) < 0) +goto cleanup; + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +if ((r = rbd_write(image, offset, length, writebuf)) < 0) { +virReportSystemError(-r, _("writing %zu bytes failed on " + "RBD image %s at offset %zu"), + length, imgname, offset); +goto cleanup; +} + +VIR_DEBUG("Wrote %zu bytes to RBD image %s at offset %zu", + length, imgname, offset); + +offset += length; +} + +ret = 0; + + cleanup: +VIR_FREE(writebuf); + +return ret; +} + +static int +virStorageBackendRBDVolWipe(virConnectPtr conn, +virStoragePoolObjPtr pool, +virStorageVolDefPtr vol, +unsigned int algorithm, +unsigned int flags) +{ +virStorageBackendRBDState ptr; +ptr.cluster = NULL; +ptr.ioctx = NULL; +rbd_image_t image = NULL; +rbd_image_info_t info; +uint64_t stripe_count; +int r = -1; +int ret = -1; + +virCheckFlags(VIR_STORAGE_VOL_WIPE_ALG_ZERO, -1); + +VIR_DEBUG("Wiping RBD image %s/%s", pool->def->source.name, vol->name); + +if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0) +goto cleanup; + +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) +goto cleanup; + +if ((r = rbd_open(ptr.ioctx, vol->name, &image, NULL)) < 0) { +virReportSystemError(-r, _("failed to open the RBD image %s"), + vol->name); +goto cleanup; +} + +if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { +virReportSystemError(-r, _("failed to stat the RBD image %s"), + vol->name); +goto cleanup; +} + +if ((r = rbd_get_stripe_count(image, &stripe_count)) < 0) { +virReportSystemError(-r, _("failed to get stripe count of RBD image %s"), + vol->name); +goto cleanup; +} + +VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s", + info.size, pool->def->source.name, vol->name); + +switch ((virStorageVolWipeAlgorithm) algorithm) { +case VIR_STORAGE_VOL_WIPE_ALG_ZERO: +r = virStorageBackendRBDVolWipeZero(image, vol->name, +info, stripe_count); +break; +default: +virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"), + algorithm); +goto cleanup; +} + +if (r < 0) { +virReportSystemError(-r, _("failed to wipe RBD image %s"), + vol->name); +goto cleanup; +} + +ret = 0; + + cleanup: +if (image) +rbd_close(image); + +virStorageBackendRBDCloseRADOSConn(&ptr); + +return ret; +} + virStorageBackend virStorageBackendRBD = { .type = VIR_STORAGE_POOL_RBD, @@ -741,4 +855,5 @@ virStorageBackend virStorageBackendRBD = { .refreshVol = virStorageBackendRBDRefreshVol, .deleteVol = virStorageBackendRBDDeleteVol, .resizeVol = virStorageBackendRBDResizeVol, +.wipeVol = virStorageBackendRBDVolWipe }; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] storage: Add TRIM algorithm to storage volume API
This new algorithm adds support for wiping volumes using TRIM. It does not overwrite all the data in a volume, but it tells the backing storage pool/driver that all bytes in a volume can be discarded. It depends on the backing storage pool how this is handled. A SCSI backend might send UNMAP commands to remove all data present on a LUN. A Ceph backend might use rbd_discard() to instruct the Ceph cluster that all data on that RBD volume can be discarded. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 3 +++ src/storage/storage_backend.c | 4 tools/virsh-volume.c | 2 +- tools/virsh.pod | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 2c55c93..232b4d3 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -153,6 +153,9 @@ typedef enum { VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ +VIR_STORAGE_VOL_WIPE_ALG_TRIM = 9, /* 1-pass, trim all data on the + volume by using TRIM or DISCARD */ + # ifdef VIR_ENUM_SENTINELS VIR_STORAGE_VOL_WIPE_ALG_LAST /* diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 15e9109..1bb44a7 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2065,6 +2065,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 | VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 | VIR_STORAGE_VOL_WIPE_ALG_RANDOM | + VIR_STORAGE_VOL_WIPE_ALG_TRIM | VIR_STORAGE_VOL_WIPE_ALG_LAST, -1); VIR_DEBUG("Wiping volume with path '%s' and algorithm %u", @@ -2112,6 +2113,9 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, case VIR_STORAGE_VOL_WIPE_ALG_RANDOM: alg_char = "random"; break; +case VIR_STORAGE_VOL_WIPE_ALG_TRIM: +alg_char = "trim"; +break; default: virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"), diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 661c876..35f0cbd 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -906,7 +906,7 @@ static const vshCmdOptDef opts_vol_wipe[] = { VIR_ENUM_DECL(virStorageVolWipeAlgorithm) VIR_ENUM_IMPL(virStorageVolWipeAlgorithm, VIR_STORAGE_VOL_WIPE_ALG_LAST, "zero", "nnsa", "dod", "bsi", "gutmann", "schneier", - "pfitzner7", "pfitzner33", "random"); + "pfitzner7", "pfitzner33", "random", "trim"); static bool cmdVolWipe(vshControl *ctl, const vshCmd *cmd) diff --git a/tools/virsh.pod b/tools/virsh.pod index e830c59..b259507 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3546,6 +3546,7 @@ B pfitzner7 - Roy Pfitzner's 7-random-pass method: random x7. pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33. random - 1-pass pattern: random. + trim - 1-pass trimming the volume using TRIM or DISCARD B: The availability of algorithms may be limited by the version of the C binary installed on the host. -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rbd: Implement buildVolFrom using RBD cloning
On 21-01-16 21:55, John Ferlan wrote: > > > On 12/23/2015 04:29 AM, Wido den Hollander wrote: >> RBD supports cloning by creating a snapshot, protecting it and create >> a child image based on that snapshot afterwards. >> >> The RBD storage driver will try to find a snapshot with zero deltas between >> the current state of the original volume and the snapshot. >> >> If such a snapshot is found a clone/child image will be created using >> the rbd_clone2() function from librbd. >> >> It will use the same features, strip size and stripe count as the parent >> image. >> >> This implementation will only create a single snapshot on the parent image if >> this never changes. That should improve performance when removing the parent >> image at some point. >> >> During build the decision will be made to user rbd_diff_iterate() or >> rbd_diff_iterate2(). >> The latter is faster, but only available on Ceph versions after 0.94 >> (Hammer). >> >> Cloning is only supported if RBD format 2 is used. All images created by >> libvirt >> are already format 2. >> >> If a RBD format 1 image is used as the original volume libvirt will return >> VIR_ERR_OPERATION_UNSUPPORTED > > Long line... and the API should return 0 or -1 (based on other > backends)... Caller doesn't expect that VIR_ERR* on return... > > I assume you realize that 'virsh vol-create-from' and 'vol-clone' end up > using the same API... > > FWIW: I reviewed bottom up - I may also have been distracted more than > once looking for specific code. Hopefully this is coherent ;-) > Thanks for all the feedback, really appreciate it! Working on a revised version of all the patches right now. Rebasing a lot, editing commits, etc, etc. The coding style changes are new to me though. I run make syntax-check and that works. Never knew that this was needed: static int virStorageRBDXXX All the exisiting code does: static int virStorageRBDXXXXXX Anyway, I'll get back with new patches. Ignore anything which is on the list right now. Btw, a Pull Request mechanism like Github would be so much easier than sending patches per e-mail ;) >> >> Signed-off-by: Wido den Hollander >> --- >> src/storage/storage_backend_rbd.c | 340 >> ++ >> 1 file changed, 340 insertions(+) >> >> diff --git a/src/storage/storage_backend_rbd.c >> b/src/storage/storage_backend_rbd.c >> index 5e16e7a..d22e5e0 100644 >> --- a/src/storage/storage_backend_rbd.c >> +++ b/src/storage/storage_backend_rbd.c >> @@ -33,6 +33,7 @@ >> #include "viruuid.h" >> #include "virstring.h" >> #include "virutil.h" >> +#include "time.h" > > Instead of time, perhaps virrandom... see note below > >> #include "rados/librados.h" >> #include "rbd/librbd.h" >> >> @@ -632,6 +633,344 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, >> return ret; >> } >> >> +static int virStorageBackendRBDImageInfo(rbd_image_t image, char *volname, >> + uint64_t *features, >> + uint64_t *stripe_unit, >> + uint64_t *stripe_count) > > static int > virStorage... > > and one line per parameter > > Also, I think this moves to help with the previous patch... > >> +{ >> +int r = -1; > > int ret = -1; > >> +uint8_t oldformat; >> + >> +r = rbd_get_old_format(image, &oldformat); >> +if (r < 0) { > > You could use (in more places than just here): > > "if ((r = rbd_*(...)) < 0) {" > > I'll just mention it once though... > >> +virReportSystemError(-r, _("failed to get the format of RBD image >> %s"), >> + volname); >> +goto cleanup; >> +} >> + >> +if (oldformat != 0) { >> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, >> + _("RBD image %s is old format. Does not support " >> + "extended features and striping"), >> + volname); >> +r = VIR_ERR_OPERATION_UNSUPPORTED; > > unnecessary > >> +goto cleanup; >> +} > > FWIW: > This is the check I'm referring to in the review of the other patch. > >> + >> +r = rbd_get_features(image, features); >> +if
[libvirt] [PATCH] rbd: Set r variable so it can be returned should an error occur
This was reported in bug #1298024 where r would be filled with the return code of rbd_open(). Should rbd_snap_unprotect() fail for any reason the virReportSystemError call would return 'Success' since rbd_open() succeeded. https://bugzilla.redhat.com/show_bug.cgi?id=1298024 Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index e20a54d..8c7a80d 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -473,7 +473,8 @@ static int virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, if (snap_count > 0) { for (i = 0; i < snap_count; i++) { -if (rbd_snap_is_protected(image, snaps[i].name, &protected)) { +r = rbd_snap_is_protected(image, snaps[i].name, &protected); +if (r < 0) { virReportSystemError(-r, _("failed to verify if snapshot '%s/%s@%s' is protected"), source->name, vol->name, snaps[i].name); @@ -485,7 +486,8 @@ static int virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, "unprotected", source->name, vol->name, snaps[i].name); -if (rbd_snap_unprotect(image, snaps[i].name) < 0) { +r = rbd_snap_unprotect(image, snaps[i].name); +if (r < 0) { virReportSystemError(-r, _("failed to unprotect snapshot '%s/%s@%s'"), source->name, vol->name, snaps[i].name); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] rbd: Add wiping RBD volumes by using rbd_discard() or rbd_write()
This allows user to use the volume wiping functionality of the libvirt storage driver. This patch also adds a new wiping algorithm VIR_STORAGE_VOL_WIPE_ALG_DISCARD By default the VIR_STORAGE_VOL_WIPE_ALG_ZERO algorithm is used and with RBD this will called rbd_write() in chunks of the underlying object size to completely zero out the volume. With VIR_STORAGE_VOL_WIPE_ALG_DISCARD it will call rbd_discard() in the same object size chunks which will trim/discard all underlying RADOS objects in the Ceph cluster. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 4 + src/storage/storage_backend_rbd.c | 157 +- tools/virsh-volume.c | 2 +- 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 2c55c93..139add3 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -153,6 +153,10 @@ typedef enum { VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ +VIR_STORAGE_VOL_WIPE_ALG_DISCARD = 9, /* 1-pass, discard all data on the + volume by using TRIM or + DISCARD */ + # ifdef VIR_ENUM_SENTINELS VIR_STORAGE_VOL_WIPE_ALG_LAST /* diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index e20a54d..c0001d0 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -32,6 +32,7 @@ #include "base64.h" #include "viruuid.h" #include "virstring.h" +#include "virutil.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -730,6 +731,159 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +static int virStorageBackendRBDVolWipeZero(rbd_image_t image, + char *imgname, + rbd_image_info_t info, + uint64_t stripe_count) +{ +int r = -1; +size_t offset = 0; +uint64_t length; +char *writebuf; + +if (VIR_ALLOC_N(writebuf, info.obj_size * stripe_count) < 0) +goto cleanup; + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +r = rbd_write(image, offset, length, writebuf); +if (r < 0) { +virReportSystemError(-r, _("writing %llu bytes failed on " + " RBD image %s at offset %llu"), + (unsigned long long)length, + imgname, + (unsigned long long)offset); +goto cleanup; +} + +VIR_DEBUG("Wrote %llu bytes to RBD image %s at offset %llu", + (unsigned long long)length, + imgname, (unsigned long long)offset); + +offset += length; +} + + cleanup: +VIR_FREE(writebuf); + +return r; +} + +static int virStorageBackendRBDVolWipeDiscard(rbd_image_t image, + char *imgname, + rbd_image_info_t info, + uint64_t stripe_count) +{ +int r = -1; +size_t offset = 0; +uint64_t length; + +VIR_DEBUG("Wiping RBD %s volume using discard)", imgname); + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +r = rbd_discard(image, offset, length); +if (r < 0) { +virReportSystemError(-r, _("discarding %llu bytes failed on " + " RBD image %s at offset %llu"), + (unsigned long long)length, + imgname, + (unsigned long long)offset); +goto cleanup; +} + +VIR_DEBUG("Discarded %llu bytes of RBD image %s at offset %llu", + (unsigned long long)length, + imgname, (unsigned long long)offset); + +offset += length; +} + + cleanup: +return r; +} + +static int virStorageBackendRBDVolWipe(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int algorithm, + unsigned int flags) +{ +virStorageBackendRBDState ptr; +ptr.cluster = NULL; +ptr.ioctx = NULL; +rbd_image_t image = NULL; +rbd_image_info_t info; +uint64_t stripe_count; +int r = -1; + +virCheckFlags(VIR_STORAG
[libvirt] [PATCH 2/4] rbd: Implement buildVolFrom using RBD cloning
RBD supports cloning by creating a snapshot, protecting it and create a child image based on that snapshot afterwards. The RBD storage driver will try to find a snapshot with zero deltas between the current state of the original volume and the snapshot. If such a snapshot is found a clone/child image will be created using the rbd_clone2() function from librbd. It will use the same features, strip size and stripe count as the parent image. This implementation will only create a single snapshot on the parent image if this never changes. That should improve performance when removing the parent image at some point. During build the decision will be made to user rbd_diff_iterate() or rbd_diff_iterate2(). The latter is faster, but only available on Ceph versions after 0.94 (Hammer). Cloning is only supported if RBD format 2 is used. All images created by libvirt are already format 2. If a RBD format 1 image is used as the original volume libvirt will return VIR_ERR_OPERATION_UNSUPPORTED Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 340 ++ 1 file changed, 340 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index c0001d0..e353be9 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -33,6 +33,7 @@ #include "viruuid.h" #include "virstring.h" #include "virutil.h" +#include "time.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -662,6 +663,344 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, return ret; } +static int virStorageBackendRBDImageInfo(rbd_image_t image, char *volname, + uint64_t *features, + uint64_t *stripe_unit, + uint64_t *stripe_count) +{ +int r = -1; +uint8_t oldformat; + +r = rbd_get_old_format(image, &oldformat); +if (r < 0) { +virReportSystemError(-r, _("failed to get the format of RBD image %s"), + volname); +goto cleanup; +} + +if (oldformat != 0) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("RBD image %s is old format. Does not support " + "extended features and striping"), + volname); +r = VIR_ERR_OPERATION_UNSUPPORTED; +goto cleanup; +} + +r = rbd_get_features(image, features); +if (r < 0) { +virReportSystemError(-r, _("failed to get the features of RBD image %s"), + volname); +goto cleanup; +} + +r = rbd_get_stripe_unit(image, stripe_unit); +if (r < 0) { +virReportSystemError(-r, _("failed to get the stripe unit of RBD image %s"), + volname); +goto cleanup; +} + +r = rbd_get_stripe_count(image, stripe_count); +if (r < 0) { +virReportSystemError(-r, _("failed to get the stripe count of RBD image %s"), + volname); +goto cleanup; +} + + cleanup: +return r; +} + +/* Callback function for rbd_diff_iterate() */ +static int virStorageBackendRBDIterateCb(uint64_t offset ATTRIBUTE_UNUSED, + size_t length ATTRIBUTE_UNUSED, + int exists ATTRIBUTE_UNUSED, + void *arg) +{ +/* + * Just set that there is a diff for this snapshot, we do not care where + */ +*(int*) arg = 1; +return -1; +} + +static int virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, char *imgname, + virBufferPtr snapname) +{ +int r = -1; +int snap_count; +int max_snaps = 128; +size_t i; +int diff; +rbd_snap_info_t *snaps = NULL; +rbd_image_info_t info; + +r = rbd_stat(image, &info, sizeof(info)); +if (r < 0) { +virReportSystemError(-r, _("failed to stat the RBD image %s"), + imgname); +goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) +VIR_FREE(snaps); + +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for RBD image %s", snap_count, imgname); + +if (snap_count == 0) { +r = -ENOENT; +goto cleanup; +} + +if (snap_count > 0) { +for (i = 0; i < snap_count; i++) { +VIR_DEBUG("Quering diff for RBD snapshot %s@%s", imgname, + snaps[i].name); + +/* The callback will set diff to
Re: [libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
On 06-01-16 14:55, John Ferlan wrote: > > > On 01/06/2016 05:36 AM, Wido den Hollander wrote: >> If no port number was provided for a storage pool libvirt defaults to >> port 6789; however, librbd/librados already default to 6789 when no port >> number is provided. >> >> In the future Ceph will switch to a new port for the Ceph monitors since >> port 6789 is already assigned to a different application by IANA. >> >> Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as >> the 'Ceph monitor' port. >> >> In this case it is the best solution to not hardcode any port number into >> libvirt and let librados handle the connection. >> >> Only if a user specifies a different port number we pass it down to librados, >> otherwise we leave it blank. >> >> Signed-off-by: Wido den Hollander >> --- >> .gnulib | 2 +- >> docs/formatdomain.html.in | 2 +- >> docs/storage.html.in | 8 +--- >> src/storage/storage_backend_rbd.c | 2 +- >> src/util/virstoragefile.c | 3 +-- >> 5 files changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/.gnulib b/.gnulib >> index 6cc32c6..f39477d 16 >> --- a/.gnulib >> +++ b/.gnulib >> @@ -1 +1 @@ >> -Subproject commit 6cc32c63e80bc1a30c521b2f07f2b54909b59892 >> +Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 > > You may want to "git submodule update --init --recursive" ;-) > Argh! That one got in during my rebase. My apologies. >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >> index ce46f06..889e721 100644 >> --- a/docs/formatdomain.html.in >> +++ b/docs/formatdomain.html.in >> @@ -2151,7 +2151,7 @@ >> rbd >> monitor servers of RBD >> one or more >> - 6789 >> + librados default >> >> >> sheepdog >> diff --git a/docs/storage.html.in b/docs/storage.html.in >> index 6c8222a..1f01330 100644 >> --- a/docs/storage.html.in >> +++ b/docs/storage.html.in >> @@ -550,7 +550,9 @@ >>backend supports cephx authentication for communication with the >>Ceph cluster. Storing the cephx authentication key is done with >>the libvirt secret mechanism. The UUID in the example pool input >> - refers to the UUID of the stored secret. >> + refers to the UUID of the stored secret. >> + The port attribute for a Ceph monitor does not have to be provided. >> + If not provided librados will use the default Ceph monitor port. >>Since 0.9.13 >> >> >> @@ -560,8 +562,8 @@ >> <name>myrbdpool</name> >> <source> >><name>rbdpool</name> >> - <host name='1.2.3.4' port='6789'/> >> - <host name='my.ceph.monitor' port='6789'/> >> + <host name='1.2.3.4'/> >> + <host name='my.ceph.monitor'/> >><host name='third.ceph.monitor' port='6789'/> >><auth username='admin' type='ceph'> >> <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> >> diff --git a/src/storage/storage_backend_rbd.c >> b/src/storage/storage_backend_rbd.c >> index 80684eb..3235a3e 100644 >> --- a/src/storage/storage_backend_rbd.c >> +++ b/src/storage/storage_backend_rbd.c >> @@ -173,7 +173,7 @@ static int >> virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, >> for (i = 0; i < source->nhost; i++) { >> if (source->hosts[i].name != NULL && >> !source->hosts[i].port) { >> -virBufferAsprintf(&mon_host, "%s:6789,", >> +virBufferAsprintf(&mon_host, "%s,", >>source->hosts[i].name); >> } else if (source->hosts[i].name != NULL && >> source->hosts[i].port) { >> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c >> index 2aa1d90..f6ba0c8 100644 >> --- a/src/util/virstoragefile.c >> +++ b/src/util/virstoragefile.c >> @@ -2246,8 +2246,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, >> if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) >> goto error; >> } else { >> -if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) >> -goto error; >> +src->hosts[src->nhosts - 1].port = NULL; > > I think perhaps somewhat ironically that since the allocation already > does a memset of 0 on the new 'hosts' element (see virExpandN), so > setting to NULL (or VIR_STRDUP'ing NULL) was superfluous. > > I've made the obvious adjustment and pushed. > Super, tnx! Wido > Tks - > > John >> } >> >> parts = virStringSplit(hostport, "\\:", 0); >> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
If no port number was provided for a storage pool libvirt defaults to port 6789; however, librbd/librados already default to 6789 when no port number is provided. In the future Ceph will switch to a new port for the Ceph monitors since port 6789 is already assigned to a different application by IANA. Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as the 'Ceph monitor' port. In this case it is the best solution to not hardcode any port number into libvirt and let librados handle the connection. Only if a user specifies a different port number we pass it down to librados, otherwise we leave it blank. Signed-off-by: Wido den Hollander --- .gnulib | 2 +- docs/formatdomain.html.in | 2 +- docs/storage.html.in | 8 +--- src/storage/storage_backend_rbd.c | 2 +- src/util/virstoragefile.c | 3 +-- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.gnulib b/.gnulib index 6cc32c6..f39477d 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 6cc32c63e80bc1a30c521b2f07f2b54909b59892 +Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ce46f06..889e721 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2151,7 +2151,7 @@ rbd monitor servers of RBD one or more - 6789 + librados default sheepdog diff --git a/docs/storage.html.in b/docs/storage.html.in index 6c8222a..1f01330 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -550,7 +550,9 @@ backend supports cephx authentication for communication with the Ceph cluster. Storing the cephx authentication key is done with the libvirt secret mechanism. The UUID in the example pool input - refers to the UUID of the stored secret. + refers to the UUID of the stored secret. + The port attribute for a Ceph monitor does not have to be provided. + If not provided librados will use the default Ceph monitor port. Since 0.9.13 @@ -560,8 +562,8 @@ <name>myrbdpool</name> <source> <name>rbdpool</name> - <host name='1.2.3.4' port='6789'/> - <host name='my.ceph.monitor' port='6789'/> + <host name='1.2.3.4'/> + <host name='my.ceph.monitor'/> <host name='third.ceph.monitor' port='6789'/> <auth username='admin' type='ceph'> <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80684eb..3235a3e 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -173,7 +173,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, for (i = 0; i < source->nhost; i++) { if (source->hosts[i].name != NULL && !source->hosts[i].port) { -virBufferAsprintf(&mon_host, "%s:6789,", +virBufferAsprintf(&mon_host, "%s,", source->hosts[i].name); } else if (source->hosts[i].name != NULL && source->hosts[i].port) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2aa1d90..f6ba0c8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2246,8 +2246,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) goto error; } else { -if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) -goto error; +src->hosts[src->nhosts - 1].port = NULL; } parts = virStringSplit(hostport, "\\:", 0); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
On 01/06/2016 09:49 AM, Andrea Bolognani wrote: > On Wed, 2016-01-06 at 09:32 +0100, Wido den Hollander wrote: >> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c >> index 2aa1d90..1354601 100644 >> --- a/src/util/virstoragefile.c >> +++ b/src/util/virstoragefile.c >> @@ -2246,7 +2246,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, >> if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) >> goto error; >> } else { >> -if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) >> +if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, NULL) < 0) >> goto error; >> } > > Shouldn't this be > >> if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) >> goto error; >> } else { >> -if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) >> -goto error; >> +src->hosts[src->nhosts - 1].port = NULL; >> } > What does that benefit? We set it to 6789 and to NULL afterwards? I just want to get rid of any hardcoded monitor ports in libvirt for Ceph. > instead? > > Cheers. > > -- > Andrea Bolognani > Software Engineer - Virtualization Team > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] rbd: Only close RBD image if it has been opened
It could be that we error out while the RBD image has not been opened yet. This would cause us to call rbd_close() on pointer which has not been initialized. Set it to NULL by default and only close if it is not NULL. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80684eb..8e2d51b 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -281,13 +281,13 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, { int ret = -1; int r = 0; -rbd_image_t image; +rbd_image_t image = NULL; r = rbd_open(ptr->ioctx, vol->name, &image, NULL); if (r < 0) { virReportSystemError(-r, _("failed to open the RBD image '%s'"), vol->name); -return ret; +goto cleanup; } rbd_image_info_t info; @@ -323,7 +323,8 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, ret = 0; cleanup: -rbd_close(image); +if (image) +rbd_close(image); return ret; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] rbd: Do not error out on a single image during pool refresh
It could happen that rbd_list() returns X names, but that while refreshing the pool one of those RBD images is removed from Ceph through a different route then libvirt. We do not need to error out in such case, we can simply ignore the volume and continue. error : volStorageBackendRBDRefreshVolInfo:289 : failed to open the RBD image 'vol-998': No such file or directory It could also be that one or more Placement Groups (PGs) inside Ceph are inactive due to a system failure. If that happens it could be that some RBD images can not be refreshed and a timeout will be raised by librados. error : volStorageBackendRBDRefreshVolInfo:289 : failed to open the RBD image 'vol-893': Connection timed out Ignore the error and continue to refresh the rest of the pool's contents. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8e2d51b..8dcb9be 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -285,6 +285,7 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, r = rbd_open(ptr->ioctx, vol->name, &image, NULL); if (r < 0) { +ret = -r; virReportSystemError(-r, _("failed to open the RBD image '%s'"), vol->name); goto cleanup; @@ -293,6 +294,7 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, rbd_image_info_t info; r = rbd_stat(image, &info, sizeof(info)); if (r < 0) { +ret = -r; virReportSystemError(-r, _("failed to stat the RBD image '%s'"), vol->name); goto cleanup; @@ -400,7 +402,21 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, name += strlen(name) + 1; -if (volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr) < 0) { +r = volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr); + +/* It could be that a volume has been deleted through a different route + * then libvirt and that will cause a -ENOENT to be returned. + * + * Another possibility is that there is something wrong with the placement + * group (PG) that RBD image's header is in and that causes -ETIMEDOUT + * to be returned. + * + * Do not error out and simply ignore the volume + */ +if (r < 0) { +if (r == -ENOENT || r == -ETIMEDOUT) +continue; + virStorageVolDefFree(vol); goto cleanup; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
If no port number was provided for a storage pool libvirt defaults to port 6789; however, librbd/librados already default to 6789 when no port number is provided. In the future Ceph will switch to a new port for the Ceph monitors since port 6789 is already assigned to a different application by IANA. Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as the 'Ceph monitor' port. In this case it is the best solution to not hardcode any port number into libvirt and let librados handle the connection. Only if a user specifies a different port number we pass it down to librados, otherwise we leave it blank. Signed-off-by: Wido den Hollander --- docs/formatdomain.html.in | 2 +- docs/storage.html.in | 8 +--- src/storage/storage_backend_rbd.c | 2 +- src/util/virstoragefile.c | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ce46f06..889e721 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2151,7 +2151,7 @@ rbd monitor servers of RBD one or more - 6789 + librados default sheepdog diff --git a/docs/storage.html.in b/docs/storage.html.in index 6c8222a..1f01330 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -550,7 +550,9 @@ backend supports cephx authentication for communication with the Ceph cluster. Storing the cephx authentication key is done with the libvirt secret mechanism. The UUID in the example pool input - refers to the UUID of the stored secret. + refers to the UUID of the stored secret. + The port attribute for a Ceph monitor does not have to be provided. + If not provided librados will use the default Ceph monitor port. Since 0.9.13 @@ -560,8 +562,8 @@ <name>myrbdpool</name> <source> <name>rbdpool</name> - <host name='1.2.3.4' port='6789'/> - <host name='my.ceph.monitor' port='6789'/> + <host name='1.2.3.4'/> + <host name='my.ceph.monitor'/> <host name='third.ceph.monitor' port='6789'/> <auth username='admin' type='ceph'> <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80684eb..3235a3e 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -173,7 +173,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, for (i = 0; i < source->nhost; i++) { if (source->hosts[i].name != NULL && !source->hosts[i].port) { -virBufferAsprintf(&mon_host, "%s:6789,", +virBufferAsprintf(&mon_host, "%s,", source->hosts[i].name); } else if (source->hosts[i].name != NULL && source->hosts[i].port) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2aa1d90..1354601 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2246,7 +2246,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) goto error; } else { -if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) +if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, NULL) < 0) goto error; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
On 01/05/2016 11:36 PM, John Ferlan wrote: > > > On 12/28/2015 10:33 AM, Wido den Hollander wrote: >> If no port number was provided for a storage pool libvirt would default >> to port 6789. >> >> librbd/librados will however already default to 6789 when no port number >> is provided. > > reads better as: > > If no port number was provided for a storage pool libvirt defaults to > port 6789; however, librbd/librados already default to 6789 when no port > number is provided. > Ah, true. I'll send a new patch with also a doc fix. >> >> In the future Ceph will however switch to a new port for the Ceph monitors > > s/Ceph will however switch/Ceph will switch/ > >> since port 6789 is already assigned to a different application by IANA. >> >> Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as >> the 'Ceph monitor' port. >> >> In this case it is the best solution to not hardcode any port number into >> libvirt and let librados handle the connection. >> >> Only if a user specifies a different port number we pass it down to librados, >> otherwise we leave it blank. >> >> Signed-off-by: Wido den Hollander >> --- >> src/storage/storage_backend_rbd.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > > Is there a specific librdb/librados that would "fail" if a port number > wasn't assigned/passed? > No, there is not. This was just something I did 'wrong' when I wrote the RBD integration a few years ago. > Think in terms of support some "older" version that some customer may > have installed. > > Also, I searched on "6789" and found: > > src/util/virstoragefile.c/virStorageSourceRBDAddHost > > docs/formatdomain.html.in > > I think in particular the docs will need to be updated... > Yes, I'll come up with a new patch. Wido > John > >> diff --git a/src/storage/storage_backend_rbd.c >> b/src/storage/storage_backend_rbd.c >> index cdbfdee..df4a3d3 100644 >> --- a/src/storage/storage_backend_rbd.c >> +++ b/src/storage/storage_backend_rbd.c >> @@ -173,7 +173,7 @@ static int >> virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, >> for (i = 0; i < source->nhost; i++) { >> if (source->hosts[i].name != NULL && >> !source->hosts[i].port) { >> -virBufferAsprintf(&mon_host, "%s:6789,", >> +virBufferAsprintf(&mon_host, "%s,", >>source->hosts[i].name); >> } else if (source->hosts[i].name != NULL && >> source->hosts[i].port) { >> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Do not error out on a single image during pool refresh
It could happen that rbd_list() returns X names, but that while refreshing the pool one of those RBD images is removed from Ceph through a different route then libvirt. We do not need to error out in such case, we can simply ignore the volume and continue. error : volStorageBackendRBDRefreshVolInfo:289 : failed to open the RBD image 'vol-998': No such file or directory It could also be that one or more Placement Groups (PGs) inside Ceph are inactive due to a system failure. If that happens it could be that some RBD images can not be refreshed and a timeout will be raised by librados. error : volStorageBackendRBDRefreshVolInfo:289 : failed to open the RBD image 'vol-893': Connection timed out Ignore the error and continue to refresh the rest of the pool's contents. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 37 - 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80684eb..888e3be 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -279,18 +279,17 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStoragePoolObjPtr pool, virStorageBackendRBDStatePtr ptr) { -int ret = -1; -int r = 0; -rbd_image_t image; +int r = -1; +rbd_image_t image = NULL; +rbd_image_info_t info; r = rbd_open(ptr->ioctx, vol->name, &image, NULL); if (r < 0) { virReportSystemError(-r, _("failed to open the RBD image '%s'"), vol->name); -return ret; +goto cleanup; } -rbd_image_info_t info; r = rbd_stat(image, &info, sizeof(info)); if (r < 0) { virReportSystemError(-r, _("failed to stat the RBD image '%s'"), @@ -308,6 +307,8 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, vol->type = VIR_STORAGE_VOL_NETWORK; vol->target.format = VIR_STORAGE_FILE_RAW; +r = -1; + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->source.name, @@ -320,11 +321,13 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, vol->name) == -1) goto cleanup; -ret = 0; +r = 0; cleanup: -rbd_close(image); -return ret; +if (image) +rbd_close(image); + +return r; } static int virStorageBackendRBDRefreshPool(virConnectPtr conn, @@ -399,7 +402,23 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, name += strlen(name) + 1; -if (volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr) < 0) { +r = volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr); + +/* It could be that a volume has been deleted through a different route + * then libvirt and that will cause a -ENOENT to be returned. + * + * Another possibility is that there is something wrong with the placement + * group (PG) that RBD image's header is in and that causes -ETIMEDOUT + * to be returned. + * + * Do not error out and simply ignore the volume + */ +if (r == -ENOENT || r == -ETIMEDOUT) { +virStorageVolDefFree(vol); +continue; +} + +if (r < 0) { virStorageVolDefFree(vol); goto cleanup; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
If no port number was provided for a storage pool libvirt would default to port 6789. librbd/librados will however already default to 6789 when no port number is provided. In the future Ceph will however switch to a new port for the Ceph monitors since port 6789 is already assigned to a different application by IANA. Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as the 'Ceph monitor' port. In this case it is the best solution to not hardcode any port number into libvirt and let librados handle the connection. Only if a user specifies a different port number we pass it down to librados, otherwise we leave it blank. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index cdbfdee..df4a3d3 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -173,7 +173,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, for (i = 0; i < source->nhost; i++) { if (source->hosts[i].name != NULL && !source->hosts[i].port) { -virBufferAsprintf(&mon_host, "%s:6789,", +virBufferAsprintf(&mon_host, "%s,", source->hosts[i].name); } else if (source->hosts[i].name != NULL && source->hosts[i].port) { -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: Add support for volume snapshots
NOTE: THIS DOES NOT WORK YET, PURE PROTOTYPE This patch adds support for creating, deleting, reverting and listing snapshots of storage volumes. This allows for snapshotting volumes managed by libvirt without talking directly to the backing storage. --- include/libvirt/libvirt-storage.h | 38 ++ src/conf/storage_conf.c | 111 + src/conf/storage_conf.h | 30 src/datatypes.h | 16 + src/driver-storage.h | 28 src/libvirt-storage.c | 143 ++ src/storage/storage_backend.h | 21 ++ src/storage/storage_driver.c | 102 +++ 8 files changed, 489 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 2c55c93..3e72fb5 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -177,6 +177,31 @@ typedef enum { VIR_STORAGE_XML_INACTIVE= (1 << 0), /* dump inactive pool/volume information */ } virStorageXMLFlags; + +/** + * virStorageVolSnap: + * + * a virStorageVolSnap is a private structure representing a storage volume snapshot + */ +typedef struct _virStorageVolSnap virStorageVolSnap; + +/** + * virStorageVolSnapPtr: + * + * a virStorageVolSnapPtr is pointer to a virStorageVolSnap private structure, this is the + * type used to reference a volume snapshot in the API. + */ +typedef virStorageVolSnap *virStorageVolSnapPtr; + +typedef struct _virStorageVolSnapInfo virStorageVolSnapInfo; + +struct _virStorageVolSnapInfo { +int type; /* virStorageVolType flags */ +unsigned long long allocated; /* Current size of snapshot in bytes */ +}; + +typedef virStorageVolSnapInfo *virStorageVolSnapInfoPtr; + /* * Get connection from pool. */ @@ -350,6 +375,19 @@ int virStorageVolWipe (virStorageVolPtr vol, int virStorageVolWipePattern(virStorageVolPtr vol, unsigned int algorithm, unsigned int flags); +virStorageVolSnapPtrvirStorageVolSnapCreate (virStorageVolPtr vol, + const char *xmldesc, + unsigned int flags); +int virStorageVolSnapDelete (virStorageVolSnapPtr snap, + unsigned int flags); +int virStorageVolSnapRevert (virStorageVolSnapPtr snap, + unsigned int flags); +int virStorageVolSnapList (virStorageVolPtr vol, + virStorageVolSnapPtr **snaps, + unsigned int flags); +int virStorageVolSnapGetInfo(virStorageVolSnapPtr snap, + virStorageVolSnapInfoPtr info, + unsigned int flags); int virStorageVolRef(virStorageVolPtr vol); int virStorageVolFree (virStorageVolPtr vol); diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9b8abea..acf9410 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -326,6 +326,18 @@ virStorageVolDefFree(virStorageVolDefPtr def) VIR_FREE(def); } +void +virStorageVolSnapDefFree(virStorageVolSnapDefPtr def) +{ +if (!def) +return; + +VIR_FREE(def->name); +VIR_FREE(def->key); + +VIR_FREE(def); +} + static void virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapterPtr adapter) { @@ -1484,6 +1496,105 @@ virStorageVolDefParseFile(virStoragePoolDefPtr pool, return virStorageVolDefParse(pool, NULL, filename, flags); } +static virStorageVolSnapDefPtr +virStorageVolSnapDefParseXML(virStoragePoolDefPtr pool, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ +virStorageVolSnapDefPtr ret; + +virCheckFlags(0, NULL); + +if (VIR_ALLOC(ret) < 0) +return NULL; + +if (pool == NULL) +return NULL; + +ret->name = virXPathString("string(./name)", ctxt); +if (ret->name == NULL) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing snapshot name element")); +goto error; +} + +ret->key = virXPathString("string(./key)", ctxt); + + + cleanup: +return ret; + + error: +virStorageVolSnapDefFree(ret); +ret = NULL; +goto cleanup; +} + +virStorageVolSnapDefPtr +virStorageVolSnapDefParseNode(virStoragePoolDefPtr pool, + xmlDocPtr x
[libvirt] Proposal for snapshotting functions of volumes
Hi, Currently libvirt is lacking support for snapshotting volumes through the storage pool mechanism. I want to add this to libvirt, but since the internals and the API design ideas of libvirt are not fully known to me I wanted to put this up for a basic review first. This patch can also be viewed on Github: https://github.com/wido/libvirt/commit/4483ab247f607250c5b3314d0a19509ef2512852 My main questions are: * Is this something we would like to see in libvirt? * If so, am I on the right track? * If so, what API calls should be different? The steps I want to take: 1. Add the new API calls 2. Add support to the RBD backend 3. Add support to virsh 4. Add support to the Java bindings The Apache CloudStack project uses libvirt intensively and I want to add as much support to libvirt as we need so that we don't do stuff inside CloudStack where libvirt might be a lot better place to do it. Any comments are very much appreciated before I start spending hours of my time on this :) Wido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Return VIR_STORAGE_FILE_RAW as format for RBD volumes
This used to return 'unkown' and that was not correct. A vol-dumpxml now returns: image3 libvirt/image3 10737418240 10737418240 libvirt/image3 The RBD driver will now error out if a different format than RAW is provided when creating a volume. Signed-off-by: Wido den Hollander --- src/conf/storage_conf.c | 3 ++- src/storage/storage_backend_rbd.c | 13 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9b8abea..7c81bab 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -220,7 +220,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, .volOptions = { .defaultFormat = VIR_STORAGE_FILE_RAW, - .formatToString = virStoragePoolFormatDiskTypeToString, + .formatFromString = virStorageVolumeFormatFromString, + .formatToString = virStorageFileFormatTypeToString, } }, {.poolType = VIR_STORAGE_POOL_SHEEPDOG, diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index cdbfdee..80684eb 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -306,6 +306,7 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, vol->target.capacity = info.size; vol->target.allocation = info.obj_size * info.num_objs; vol->type = VIR_STORAGE_VOL_NETWORK; +vol->target.format = VIR_STORAGE_FILE_RAW; VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", @@ -557,6 +558,12 @@ virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, { vol->type = VIR_STORAGE_VOL_NETWORK; +if (vol->target.format != VIR_STORAGE_FILE_RAW) { +virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("only RAW volumes are supported by this storage pool")); +return -VIR_ERR_NO_SUPPORT; +} + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->source.name, @@ -603,6 +610,12 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, goto cleanup; } +if (vol->target.format != VIR_STORAGE_FILE_RAW) { +virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("only RAW volumes are supported by this storage pool")); +goto cleanup; +} + if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0) goto cleanup; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] rbd: Add wiping RBD volumes by using rbd_discard() or rbd_write()
This allows user to use the volume wiping functionality of the libvirt storage driver. This patch also adds a new wiping algorithm VIR_STORAGE_VOL_WIPE_ALG_DISCARD By default the VIR_STORAGE_VOL_WIPE_ALG_ZERO algorithm is used and with RBD this will called rbd_write() in chunks of the underlying object size to completely zero out the volume. With VIR_STORAGE_VOL_WIPE_ALG_DISCARD it will call rbd_discard() in the same object size chunks which will trim/discard all underlying RADOS objects in the Ceph cluster. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 4 + src/storage/storage_backend_rbd.c | 155 +- tools/virsh-volume.c | 2 +- 3 files changed, 159 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 2c55c93..139add3 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -153,6 +153,10 @@ typedef enum { VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ +VIR_STORAGE_VOL_WIPE_ALG_DISCARD = 9, /* 1-pass, discard all data on the + volume by using TRIM or + DISCARD */ + # ifdef VIR_ENUM_SENTINELS VIR_STORAGE_VOL_WIPE_ALG_LAST /* diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index cdbfdee..d13658d 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -32,6 +32,7 @@ #include "base64.h" #include "viruuid.h" #include "virstring.h" +#include "virutil.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -700,6 +701,157 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +static int virStorageBackendRBDVolWipeZero(rbd_image_t image, + char *imgname, + rbd_image_info_t info, + uint64_t stripe_count) +{ +int r = -1; +size_t offset = 0; +uint64_t length; +char *writebuf; + +if (VIR_ALLOC_N(writebuf, info.obj_size * stripe_count) < 0) +goto cleanup; + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +r = rbd_write(image, offset, length, writebuf); +if (r < 0) { +virReportSystemError(-r, _("writing %llu bytes failed on " + " RBD image %s at offset %llu"), + (unsigned long long)length, + imgname, + (unsigned long long)offset); +goto cleanup; +} + +VIR_DEBUG("Wrote %llu bytes to RBD image %s at offset %llu", + (unsigned long long)length, + imgname, (unsigned long long)offset); + +offset += length; +} + + cleanup: +return r; +} + +static int virStorageBackendRBDVolWipeDiscard(rbd_image_t image, + char *imgname, + rbd_image_info_t info, + uint64_t stripe_count) +{ +int r = -1; +size_t offset = 0; +uint64_t length; + +VIR_DEBUG("Wiping RBD %s volume using discard)", imgname); + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +r = rbd_discard(image, offset, length); +if (r < 0) { +virReportSystemError(-r, _("discarding %llu bytes failed on " + " RBD image %s at offset %llu"), + (unsigned long long)length, + imgname, + (unsigned long long)offset); +goto cleanup; +} + +VIR_DEBUG("Discarded %llu bytes of RBD image %s at offset %llu", + (unsigned long long)length, + imgname, (unsigned long long)offset); + +offset += length; +} + + cleanup: +return r; +} + +static int virStorageBackendRBDVolWipe(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int algorithm, + unsigned int flags) +{ +virStorageBackendRBDState ptr; +ptr.cluster = NULL; +ptr.ioctx = NULL; +rbd_image_t image = NULL; +rbd_image_info_t info; +uint64_t stripe_count; +int r = -1; + +virCheckFlags(VIR_STORAGE_VOL_WIPE_ALG_ZERO | +
Re: [libvirt] [PATCH 1/2] rbd: Implement wiping RBD volumes using rbd_discard()
On 23-12-15 14:14, Daniel P. Berrange wrote: > On Wed, Dec 23, 2015 at 02:10:28PM +0100, Wido den Hollander wrote: >> >> >> On 23-12-15 14:05, Daniel P. Berrange wrote: >>> On Wed, Dec 23, 2015 at 01:50:52PM +0100, Wido den Hollander wrote: >>>> >>>> >>>> On 23-12-15 10:45, Daniel P. Berrange wrote: >>>>> On Wed, Dec 23, 2015 at 10:29:04AM +0100, Wido den Hollander wrote: >>>>>> This allows user to use the volume wiping functionality of the libvirt >>>>>> storage driver. >>>>>> >>>>>> All data from the volume will be wiped by calling rbd_discard() in >>>>>> chunks of the >>>>>> underlying object and stripe size inside Ceph. >>>>>> >>>>>> This will ensure all data is zeroed and leaves the user with a >>>>>> completely empty volume >>>>>> ready for use again. >>>>> >>>>> Based on the name 'rbd_discard' it sounds like this is going to call >>>>> TRIM/DISCARD on the underlying storage too ? If so, then I don't think >>>>> that this is an appropriate approach for this API. The virStorageVolWipe >>>>> API should clear the data, *without* having any effect on the storage of >>>>> the API - ie we don't want to discard underling storage blocks as a >>>>> side effect >>>>> >>>> >>>> Afaik it zeroes/trims all the RBD objects on the Ceph cluster, but it >>>> doesn't TRIM the lower SSD on it's turn. >>>> >>>> So it will send these calls to Ceph/RBD and it will zero all the data of >>>> that specific volume. A rather simple way to get rid of the data in a >>>> volume. >>> >>> That's not what I see that API impl of rbd_discard() doing. It >>> looks very much like it is discarding extents from the RBD volume, >>> at least if the discarded region is large enough. Only if the discarded >>> region is small, does it merely zero the data. >>> >> >> Let me verify this with the Ceph people. >> >>> So I really don't think this is a suitable API for use with the >>> virStorageVolWipe() API, whose *only* effect should be to overwrite >>> the data, not have any side effect on volume extent allocation >>> >> >> What do you suggest? use rbd_write() in a loop and overwrite the whole >> volume? > > That would match intended semantics of WIPE_ALG_ZERO albeit > rather slow > >> The problem with this option is that the RBD volume will then grow to >> it's maximum size on the underlying storage. > > This is the same semantics as we have for the other impls of the > Wipe API. The API is defined to fill the contents of the file > with the requested byte pattern, which neccessarily means that > the volume will grow to its maximum size. > Ah, ok. I misunderstood that. >> With rbd_discard() being called on the exact object size all those >> objects will be trimmed, effectively wiping the volume's data. So I >> figured it was OK. > > We could introduce a VIR_STORAGE_VOL_WIPE_ALG_DISCARD which is > defined to be the same as VIR_STORAGE_VOL_WIPE_ALG_ZERO, but > using discard semantics to release space. > That seems fine with me. What would the best way be to proceed? Simply add the flag and that's it? This part of libvirt is not really my knowledge. Thanks for the super fast responses btw! Wido > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] rbd: Implement wiping RBD volumes using rbd_discard()
On 23-12-15 14:05, Daniel P. Berrange wrote: > On Wed, Dec 23, 2015 at 01:50:52PM +0100, Wido den Hollander wrote: >> >> >> On 23-12-15 10:45, Daniel P. Berrange wrote: >>> On Wed, Dec 23, 2015 at 10:29:04AM +0100, Wido den Hollander wrote: >>>> This allows user to use the volume wiping functionality of the libvirt >>>> storage driver. >>>> >>>> All data from the volume will be wiped by calling rbd_discard() in chunks >>>> of the >>>> underlying object and stripe size inside Ceph. >>>> >>>> This will ensure all data is zeroed and leaves the user with a completely >>>> empty volume >>>> ready for use again. >>> >>> Based on the name 'rbd_discard' it sounds like this is going to call >>> TRIM/DISCARD on the underlying storage too ? If so, then I don't think >>> that this is an appropriate approach for this API. The virStorageVolWipe >>> API should clear the data, *without* having any effect on the storage of >>> the API - ie we don't want to discard underling storage blocks as a >>> side effect >>> >> >> Afaik it zeroes/trims all the RBD objects on the Ceph cluster, but it >> doesn't TRIM the lower SSD on it's turn. >> >> So it will send these calls to Ceph/RBD and it will zero all the data of >> that specific volume. A rather simple way to get rid of the data in a >> volume. > > That's not what I see that API impl of rbd_discard() doing. It > looks very much like it is discarding extents from the RBD volume, > at least if the discarded region is large enough. Only if the discarded > region is small, does it merely zero the data. > Let me verify this with the Ceph people. > So I really don't think this is a suitable API for use with the > virStorageVolWipe() API, whose *only* effect should be to overwrite > the data, not have any side effect on volume extent allocation > What do you suggest? use rbd_write() in a loop and overwrite the whole volume? The problem with this option is that the RBD volume will then grow to it's maximum size on the underlying storage. With rbd_discard() being called on the exact object size all those objects will be trimmed, effectively wiping the volume's data. So I figured it was OK. I just want to make the RBD storage driver as feature complete as possible :) Wido > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] rbd: Implement wiping RBD volumes using rbd_discard()
On 23-12-15 10:45, Daniel P. Berrange wrote: > On Wed, Dec 23, 2015 at 10:29:04AM +0100, Wido den Hollander wrote: >> This allows user to use the volume wiping functionality of the libvirt >> storage driver. >> >> All data from the volume will be wiped by calling rbd_discard() in chunks of >> the >> underlying object and stripe size inside Ceph. >> >> This will ensure all data is zeroed and leaves the user with a completely >> empty volume >> ready for use again. > > Based on the name 'rbd_discard' it sounds like this is going to call > TRIM/DISCARD on the underlying storage too ? If so, then I don't think > that this is an appropriate approach for this API. The virStorageVolWipe > API should clear the data, *without* having any effect on the storage of > the API - ie we don't want to discard underling storage blocks as a > side effect > Afaik it zeroes/trims all the RBD objects on the Ceph cluster, but it doesn't TRIM the lower SSD on it's turn. So it will send these calls to Ceph/RBD and it will zero all the data of that specific volume. A rather simple way to get rid of the data in a volume. If makes sure that the volume no longer contains any data for the end-user. All user-data on the volume will be gone. Or am I misunderstanding you? What is it supposed to do? Wido > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] rbd: Add support for wiping and cloning images to storage driver
These two patches add support for buildVolFrom and wipeVol to the RBD storage driver. Wiping is done by discarding all blocks on the RBD image. Building a volume from a original is done leveraging the cloning mechanism RBD already supports. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] rbd: Implement wiping RBD volumes using rbd_discard()
This allows user to use the volume wiping functionality of the libvirt storage driver. All data from the volume will be wiped by calling rbd_discard() in chunks of the underlying object and stripe size inside Ceph. This will ensure all data is zeroed and leaves the user with a completely empty volume ready for use again. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 90 ++- 1 file changed, 89 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index cdbfdee..5e16e7a 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -32,6 +32,7 @@ #include "base64.h" #include "viruuid.h" #include "virstring.h" +#include "virutil.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -700,6 +701,92 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +static int virStorageBackendRBDVolWipe(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int algorithm, + unsigned int flags) +{ +virStorageBackendRBDState ptr; +ptr.cluster = NULL; +ptr.ioctx = NULL; +rbd_image_t image = NULL; +rbd_image_info_t info; +int r = -1; +size_t offset = 0; +uint64_t stripe_count; +uint64_t length; + +virCheckFlags(0, -1); + +VIR_DEBUG("Wiping RBD image %s/%s", pool->def->source.name, vol->name); + +if (algorithm != VIR_STORAGE_VOL_WIPE_ALG_ZERO) { +virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("RBD storage pool does not support other wiping" + " algorithms than zero")); +goto cleanup; +} + +if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0) +goto cleanup; + +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) +goto cleanup; + +r = rbd_open(ptr.ioctx, vol->name, &image, NULL); +if (r < 0) { +virReportSystemError(-r, _("failed to open the RBD image %s"), + vol->name); +goto cleanup; +} + +r = rbd_stat(image, &info, sizeof(info)); +if (r < 0) { +virReportSystemError(-r, _("failed to stat the RBD image %s"), + vol->name); +goto cleanup; +} + +r = rbd_get_stripe_count(image, &stripe_count); +if (r < 0) { +virReportSystemError(-r, _("failed to get stripe count of RBD image %s"), + vol->name); +goto cleanup; +} + +VIR_DEBUG("Need to wipe %llu bytes from RBD image %s/%s", + (unsigned long long)info.size, pool->def->source.name, vol->name); + +while (offset < info.size) { +length = MIN((info.size - offset), (info.obj_size * stripe_count)); + +r = rbd_discard(image, offset, length); +if (r < 0) { +virReportSystemError(-r, _("discarding %llu bytes failed on " + " RBD image %s at offset %llu"), + (unsigned long long)length, + vol->name, + (unsigned long long)offset); +goto cleanup; +} + +VIR_DEBUG("Discarded %llu bytes of RBD image %s/%s at offset %llu", + (unsigned long long)length, + pool->def->source.name, + vol->name, (unsigned long long)offset); + +offset += length; +} + + cleanup: +if (image) +rbd_close(image); + +virStorageBackendRBDCloseRADOSConn(&ptr); +return r; +} + virStorageBackend virStorageBackendRBD = { .type = VIR_STORAGE_POOL_RBD, @@ -708,5 +795,6 @@ virStorageBackend virStorageBackendRBD = { .buildVol = virStorageBackendRBDBuildVol, .refreshVol = virStorageBackendRBDRefreshVol, .deleteVol = virStorageBackendRBDDeleteVol, -.resizeVol = virStorageBackendRBDResizeVol, +.wipeVol = virStorageBackendRBDVolWipe, +.resizeVol = virStorageBackendRBDResizeVol }; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] rbd: Implement buildVolFrom using RBD cloning
RBD supports cloning by creating a snapshot, protecting it and create a child image based on that snapshot afterwards. The RBD storage driver will try to find a snapshot with zero deltas between the current state of the original volume and the snapshot. If such a snapshot is found a clone/child image will be created using the rbd_clone2() function from librbd. It will use the same features, strip size and stripe count as the parent image. This implementation will only create a single snapshot on the parent image if this never changes. That should improve performance when removing the parent image at some point. During build the decision will be made to user rbd_diff_iterate() or rbd_diff_iterate2(). The latter is faster, but only available on Ceph versions after 0.94 (Hammer). Cloning is only supported if RBD format 2 is used. All images created by libvirt are already format 2. If a RBD format 1 image is used as the original volume libvirt will return VIR_ERR_OPERATION_UNSUPPORTED Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 340 ++ 1 file changed, 340 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5e16e7a..d22e5e0 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -33,6 +33,7 @@ #include "viruuid.h" #include "virstring.h" #include "virutil.h" +#include "time.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -632,6 +633,344 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, return ret; } +static int virStorageBackendRBDImageInfo(rbd_image_t image, char *volname, + uint64_t *features, + uint64_t *stripe_unit, + uint64_t *stripe_count) +{ +int r = -1; +uint8_t oldformat; + +r = rbd_get_old_format(image, &oldformat); +if (r < 0) { +virReportSystemError(-r, _("failed to get the format of RBD image %s"), + volname); +goto cleanup; +} + +if (oldformat != 0) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("RBD image %s is old format. Does not support " + "extended features and striping"), + volname); +r = VIR_ERR_OPERATION_UNSUPPORTED; +goto cleanup; +} + +r = rbd_get_features(image, features); +if (r < 0) { +virReportSystemError(-r, _("failed to get the features of RBD image %s"), + volname); +goto cleanup; +} + +r = rbd_get_stripe_unit(image, stripe_unit); +if (r < 0) { +virReportSystemError(-r, _("failed to get the stripe unit of RBD image %s"), + volname); +goto cleanup; +} + +r = rbd_get_stripe_count(image, stripe_count); +if (r < 0) { +virReportSystemError(-r, _("failed to get the stripe count of RBD image %s"), + volname); +goto cleanup; +} + + cleanup: +return r; +} + +/* Callback function for rbd_diff_iterate() */ +static int virStorageBackendRBDIterateCb(uint64_t offset ATTRIBUTE_UNUSED, + size_t length ATTRIBUTE_UNUSED, + int exists ATTRIBUTE_UNUSED, + void *arg) +{ +/* + * Just set that there is a diff for this snapshot, we do not care where + */ +*(int*) arg = 1; +return -1; +} + +static int virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, char *imgname, + virBufferPtr snapname) +{ +int r = -1; +int snap_count; +int max_snaps = 128; +size_t i; +int diff; +rbd_snap_info_t *snaps = NULL; +rbd_image_info_t info; + +r = rbd_stat(image, &info, sizeof(info)); +if (r < 0) { +virReportSystemError(-r, _("failed to stat the RBD image %s"), + imgname); +goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) +VIR_FREE(snaps); + +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for RBD image %s", snap_count, imgname); + +if (snap_count == 0) { +r = -ENOENT; +goto cleanup; +} + +if (snap_count > 0) { +for (i = 0; i < snap_count; i++) { +VIR_DEBUG("Quering diff for RBD snapshot %s@%s", imgname, + snaps[i].name); + +/* The callback will set diff to
Re: [libvirt] [PATCH 0/3] Add delete-snapshots option to virsh commands
On 12/17/2015 09:54 PM, John Ferlan wrote: > > ping? > Looks good to me. Didn't think you needed a reply from me. But yes, I forgot this part. Only went for the API since that was what I needed in CloudStack. > Thanks - > > John > > On 12/02/2015 06:18 PM, John Ferlan wrote: >> Commit id '3c7590e0' added the ability to delete snapshots for rbd backends, >> but did not provide the virsh commands in order to handle that option. >> >> The first two patches fix a couple of minor nits - not using virCheckFlags >> in virStorageBackendRBDDeleteVol even though the flags are used. Also, >> the virStorageVolDeleteFlags did not document the possible flags. Added >> the reference for the API. >> >> The third patch is the meat where the --delete-snapshots flag to virsh >> commands 'undefine' and 'vol-delete' was added and handled. >> >> cc'd author of '3c7590e0' - hopefully this series could be applied and >> tested in a "real" environment. >> >> John Ferlan (3): >> storage: Add virCheckFlags to virStorageBackendRBDDeleteVol >> libvirt: Add virStorageVolDeleteFlags to virStorageVolDelete >> virsh: Add --delete-snapshots flag for undefine and vol-delete >> >> src/libvirt-storage.c | 2 +- >> src/storage/storage_backend_rbd.c | 3 +++ >> tools/virsh-domain.c | 14 +- >> tools/virsh-volume.c | 12 +++- >> tools/virsh.pod | 14 +- >> 5 files changed, 41 insertions(+), 4 deletions(-) >> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH java] Add support for Libvirt Qemu Library
On 11/17/2015 08:43 AM, Claudio Bley wrote: > At Mon, 16 Nov 2015 21:26:57 +0100, > Wido den Hollander wrote: >> >> >> >>> Since there are no flags currently, simply drop the flags >>> parameter. We add flags (as enums) later when the function starts to >>> support one. >>> >> >> Good point, I'll come up with a revised patch. >> >>> Furthermore, I think I'm in favor of adding a handful of methods >>> instead of (ab)using some special values of a parameter. >>> >>> /* blocking call */ >>> String qemuAgentCommand(String cmd) >>> >>> /* default timeout call */ >>> String qemuAgentCommandDefaultTimeout(String cmd) >>> >>> /* timeout in seconds. no special 0 constant needed. */ >>> String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) { >>> if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds >>> value must be >= 0"); >>> >>> ... >>> } >>> >>> Do you have a link to some docs where the available commands are >>> described? We should add this to the javadoc. >>> >> >> Yes, here they are: http://wiki.qemu.org/Features/QAPI/GuestAgent >> >>> What does a qemu command look like? It seems it's some kind of JSON >>> object. In that case, I think we should not make every user of the >>> library start to construct their own JSON strings. >>> >>> May be a simple utility class, e.g. >>> >>> public class QemuCommand { >>> public static QemuCommand create(String cmd, String args...) { >>> ... >>> } >>> } >>> >>> should be added and the qemuAgentCommand changed to take such a >>> parameter instead? >>> >> >> Hmm, I don't know if we want to do that. I think we should always keep >> the option open for the user. Libvirt doesn't provide it. We are only >> mapping libvirt functions. > > Yes, but that doesn't mean we should not improve it, IMO. Thankfully, > in Java, we're not bound by the limitations of the C language. Ease of > use of the API is one of my foremost goals for libvirt-java. A plain > mapping of the C API functions would be far easier, but also downright > ugly (with the Java goggles on). > > Having a (bit) of Haskell background, and being a Scala programmer by > day, a plain String in place of a proper type is just ugly as well, as > it does not carry any information as to what it's type really is and > what it's format is. > > I see that the JSON structure of an agent command is pretty simple, > but we would have to support strings and numbers. I'd still very much > prefer a QemuCommand helper class. > True, I understand. Looking at this I would want to use the JSONReader/JsonWriter from Java, but that's not available by default. We probably don't want any external libs for libvirt-java. 'arguments' for Qemu GA is rather simple, it's an array with Strings or Ints. I rather not write my own JsonWriter, so any ideas here? > If it turns out that it is not flexible enough, we can always add a > loophole later, e.g.: > Indeed. We could also implement all available commands as a Enum: http://git.qemu.org/?p=qemu.git;a=blob;f=qga/qapi-schema.json;h=78362e071d18de1f04f3b5730d8a32dcb46ee74c;hb=HEAD It's just the arguments I'm stuck with. I have a few commits, do you care to take a look at them: https://github.com/wido/libvirt-java/commits/qemu-guest-command Especially this one: https://github.com/wido/libvirt-java/commit/1ff76ed0a8f8d58a102cd4e2bc948df99a472e20 Wido > class QemuCommand { > public static QemuCommand execute(String cmd, /* TBD */ args...) { > } > > public static QemuCommand raw(String jsonObject) { > } > } > >>>> Yes, that would indeed fail. What do you suggest? Load when required? My >>>> JNA foo is not that good to get this implemented. >>> >>> We can probably use some sort of singleton pattern >>> (e.g. initialization-on-demand holder). >>> >> >> I might need some help with that! I'll come up with a revised patched >> based on your comments. > > OK, I'm sure we can work that out. Basically, we probably need to > access the qemu library through a static method, so we avoid loading > the library when it is not necessary. Let me think about that a bit > more. > > -- > Claudio-- > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH java] Add support for Libvirt Qemu Library
On 11/15/2015 11:26 PM, Claudio Bley wrote: > At Fri, 13 Nov 2015 14:59:52 +0100, > Wido den Hollander wrote: >> >> >> >> On 12-11-15 23:04, Claudio Bley wrote: >>> Hi. >>> >>> At Mon, 9 Nov 2015 13:48:18 +0100, >>> Wido den Hollander wrote: >>>> >>>> This allows us to send Qemu Guest Agent commands to running domains. >>>> >>>> Signed-off-by: Wido den Hollander >>>> --- >>>> src/main/java/org/libvirt/Domain.java | 36 >>>> ++ >>>> src/main/java/org/libvirt/Library.java | 3 +++ >>>> src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 >>>> 3 files changed, 55 insertions(+) >>>> create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java >>>> >>>> diff --git a/src/main/java/org/libvirt/Domain.java >>>> b/src/main/java/org/libvirt/Domain.java >>>> index 83a500c..c24df48 100644 >>>> --- a/src/main/java/org/libvirt/Domain.java >>>> +++ b/src/main/java/org/libvirt/Domain.java >>>> @@ -141,6 +143,23 @@ public class Domain { >>>> public static final int NO_METADATA = (1 << 4); >>>> } >>>> >>>> +static final class QemuAgentFlags { >>>> +/** >>>> + * Do not wait for a result >>>> + */ >>>> +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0; >>>> + >>>> +/** >>>> + * Use default timeout value >>>> + */ >>>> +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = >>>> -1; >>>> + >>>> +/** >>>> + * Block forever waiting for a result >>>> + */ >>>> +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2; >>>> +} >>>> + >>> >>> I would much prefer an Enum instead of some integer constants. Also, >>> those flags are currently of little use since the QemuAgentFlags class >>> is package private. >>> >>> Also, which version are you targeting? Seems there's also a shutdown flag: >>> >>> typedef enum { >>> VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2, >>> VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2, >>> VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1, >>> VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0, >>> VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60, >>> } virDomainQemuAgentCommandTimeoutValues; >>> >>> And why are those flags mixed up? Seems like a bad idea to me. Are >>> those "values" intended to go into the "timeout" parameter or into the >>> flags? >>> >>> And since which release is this actually available in libvirt? >>> >> >> Hmm, I looked at the master branch on Git. I didn't find the ones you >> send. Let me work on that. > > They are defined in include/libvirt/libvirt-qemu.h. > > Apparently, those constants are special timeout values and the > function does not take any flags currently. So the name > "QemuAgentFlags" was a bit misleading. > > The SHUTDOWN value is merely an internal value in case when you want > to shut down the guest via the agent, basically an educated guess that > a 60 seconds timeout value should work for a shut down in most cases. > > See dd725c53e905db51f39dbaa4a0673e8d1588301b > > Since there are no flags currently, simply drop the flags > parameter. We add flags (as enums) later when the function starts to > support one. > Good point, I'll come up with a revised patch. > Furthermore, I think I'm in favor of adding a handful of methods > instead of (ab)using some special values of a parameter. > > /* blocking call */ > String qemuAgentCommand(String cmd) > > /* default timeout call */ > String qemuAgentCommandDefaultTimeout(String cmd) > > /* timeout in seconds. no special 0 constant needed. */ > String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) { > if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds > value must be >= 0"); > > ... > } > > Do you have a link to some docs where the available commands are > described? We should add this to the javadoc. > Yes, here they are: http://wiki.qemu.org/Features/QAPI/GuestAgent > What does a qemu command look like? It seems it's some kind of JSON > object. In t
Re: [libvirt] [PATCH java] Add support for Libvirt Qemu Library
On 12-11-15 23:04, Claudio Bley wrote: > Hi. > > At Mon, 9 Nov 2015 13:48:18 +0100, > Wido den Hollander wrote: >> >> This allows us to send Qemu Guest Agent commands to running domains. >> >> Signed-off-by: Wido den Hollander >> --- >> src/main/java/org/libvirt/Domain.java | 36 >> ++ >> src/main/java/org/libvirt/Library.java | 3 +++ >> src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 >> 3 files changed, 55 insertions(+) >> create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java >> >> diff --git a/src/main/java/org/libvirt/Domain.java >> b/src/main/java/org/libvirt/Domain.java >> index 83a500c..c24df48 100644 >> --- a/src/main/java/org/libvirt/Domain.java >> +++ b/src/main/java/org/libvirt/Domain.java >> @@ -141,6 +143,23 @@ public class Domain { >> public static final int NO_METADATA = (1 << 4); >> } >> >> +static final class QemuAgentFlags { >> +/** >> + * Do not wait for a result >> + */ >> +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0; >> + >> +/** >> + * Use default timeout value >> + */ >> +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1; >> + >> +/** >> + * Block forever waiting for a result >> + */ >> +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2; >> +} >> + > > I would much prefer an Enum instead of some integer constants. Also, > those flags are currently of little use since the QemuAgentFlags class > is package private. > > Also, which version are you targeting? Seems there's also a shutdown flag: > > typedef enum { > VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2, > VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2, > VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1, > VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0, > VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60, > } virDomainQemuAgentCommandTimeoutValues; > > And why are those flags mixed up? Seems like a bad idea to me. Are > those "values" intended to go into the "timeout" parameter or into the > flags? > > And since which release is this actually available in libvirt? > Hmm, I looked at the master branch on Git. I didn't find the ones you send. Let me work on that. >> diff --git a/src/main/java/org/libvirt/Library.java >> b/src/main/java/org/libvirt/Library.java >> index 8e054c6..30f15be 100644 >> --- a/src/main/java/org/libvirt/Library.java >> +++ b/src/main/java/org/libvirt/Library.java >> @@ -2,6 +2,7 @@ package org.libvirt; >> >> import org.libvirt.jna.Libvirt; >> import org.libvirt.jna.Libvirt.VirEventTimeoutCallback; >> +import org.libvirt.jna.LibvirtQemu; >> import org.libvirt.jna.CString; >> import static org.libvirt.ErrorHandler.processError; >> >> @@ -34,6 +35,7 @@ public final class Library { >> }; >> >> final static Libvirt libvirt; >> +final static LibvirtQemu libvirtqemu; >> >> // an empty string array constant >> // prefer this over creating empty arrays dynamically. >> @@ -47,6 +49,7 @@ public final class Library { >> } catch (Exception e) { >> e.printStackTrace(); >> } >> +libvirtqemu = LibvirtQemu.INSTANCE; >> } > > I doubt that we always should load that library. In case the user has > an old version this would fail, wouldn't it? > Yes, that would indeed fail. What do you suggest? Load when required? My JNA foo is not that good to get this implemented. Wido > -- > Claudio > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH java] Add support for Libvirt Qemu Library
This allows us to send Qemu Guest Agent commands to running domains. Signed-off-by: Wido den Hollander --- src/main/java/org/libvirt/Domain.java | 36 ++ src/main/java/org/libvirt/Library.java | 3 +++ src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 3 files changed, 55 insertions(+) create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index 83a500c..c24df48 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -8,6 +8,7 @@ import org.libvirt.jna.CString; import org.libvirt.jna.DomainPointer; import org.libvirt.jna.DomainSnapshotPointer; import org.libvirt.jna.Libvirt; +import org.libvirt.jna.LibvirtQemu; import org.libvirt.jna.SizeT; import org.libvirt.jna.virDomainBlockInfo; import org.libvirt.jna.virDomainBlockStats; @@ -22,6 +23,7 @@ import org.libvirt.event.LifecycleListener; import org.libvirt.event.PMWakeupListener; import org.libvirt.event.PMSuspendListener; import static org.libvirt.Library.libvirt; +import static org.libvirt.Library.libvirtqemu; import static org.libvirt.ErrorHandler.processError; import static org.libvirt.ErrorHandler.processErrorIfZero; @@ -141,6 +143,23 @@ public class Domain { public static final int NO_METADATA = (1 << 4); } +static final class QemuAgentFlags { +/** + * Do not wait for a result + */ +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0; + +/** + * Use default timeout value + */ +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1; + +/** + * Block forever waiting for a result + */ +public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2; +} + /** * the native virDomainPtr. */ @@ -1556,4 +1575,21 @@ public class Domain { return processError(libvirt.virDomainUpdateDeviceFlags(VDP, xml, flags)); } +/** + * Send a Qemu Guest Agent command to a domain + * + * @param cmd + *The command which has to be send + * @param timeout + *The timeout for waiting on an answer. See QemuAgentFlags for more information. + * @param flags + *Flags to be send + * @return String + * @throws LibvirtException + */ +public String qemuAgentCommand(String cmd, int timeout, int flags) throws LibvirtException { +CString result = libvirtqemu.virDomainQemuAgentCommand(this.VDP, cmd, timeout, flags); +processError(result); +return result.toString(); +} } diff --git a/src/main/java/org/libvirt/Library.java b/src/main/java/org/libvirt/Library.java index 8e054c6..30f15be 100644 --- a/src/main/java/org/libvirt/Library.java +++ b/src/main/java/org/libvirt/Library.java @@ -2,6 +2,7 @@ package org.libvirt; import org.libvirt.jna.Libvirt; import org.libvirt.jna.Libvirt.VirEventTimeoutCallback; +import org.libvirt.jna.LibvirtQemu; import org.libvirt.jna.CString; import static org.libvirt.ErrorHandler.processError; @@ -34,6 +35,7 @@ public final class Library { }; final static Libvirt libvirt; +final static LibvirtQemu libvirtqemu; // an empty string array constant // prefer this over creating empty arrays dynamically. @@ -47,6 +49,7 @@ public final class Library { } catch (Exception e) { e.printStackTrace(); } +libvirtqemu = LibvirtQemu.INSTANCE; } private Library() {} diff --git a/src/main/java/org/libvirt/jna/LibvirtQemu.java b/src/main/java/org/libvirt/jna/LibvirtQemu.java new file mode 100644 index 000..82213e9 --- /dev/null +++ b/src/main/java/org/libvirt/jna/LibvirtQemu.java @@ -0,0 +1,16 @@ +package org.libvirt.jna; + +import com.sun.jna.Library; +import com.sun.jna.Native; +import com.sun.jna.Platform; + +/** + * The libvirt Qemu interface which is exposed via JNA + */ + +public interface LibvirtQemu extends Library { + +LibvirtQemu INSTANCE = (LibvirtQemu) Native.loadLibrary(Platform.isWindows() ? "virt-qemu-0" : "virt-qemu", LibvirtQemu.class); + +CString virDomainQemuAgentCommand(DomainPointer virDomainPtr, String cmd, int timeout, int flags); +} -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-java] Add new WITH_SNAPSHOTS removal flag for Storage Volumes
This was recently added to libvirt and this is to match the API Signed-off-by: Wido den Hollander --- src/main/java/org/libvirt/StorageVol.java | 4 1 file changed, 4 insertions(+) diff --git a/src/main/java/org/libvirt/StorageVol.java b/src/main/java/org/libvirt/StorageVol.java index 3b1533e..47a9013 100644 --- a/src/main/java/org/libvirt/StorageVol.java +++ b/src/main/java/org/libvirt/StorageVol.java @@ -22,6 +22,10 @@ public class StorageVol { * Clear all data to zeros (slow) */ static final int VIR_STORAGE_POOL_DELETE_ZEROED = 1; +/** + * Delete a volume even if it has snapshots + */ +static final int VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 2; } public static final class ResizeFlags { -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Remove snapshots if the DELETE_WITH_SNAPSHOTS flag has been provided
When a RBD volume has snapshots it can not be removed. This patch introduces a new flag to force volume removal, VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS. With this flag any existing snapshots will be removed prior to removing the volume. No existing mechanism in libvirt allowed us to pass such information, so that's why a new flag was introduced. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 1 + src/storage/storage_backend_rbd.c | 89 +++ 2 files changed, 90 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 453089e..9fc3c2d 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -115,6 +115,7 @@ typedef enum { typedef enum { VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */ VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros (slow) */ +VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 1 << 1, /* Force removal of volume, even if in use */ } virStorageVolDeleteFlags; typedef enum { diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5ae4713..a37d286 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -421,6 +421,87 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, return ret; } +static int virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, +virStoragePoolSourcePtr source, +virStorageVolDefPtr vol) +{ +int ret = -1; +int r = 0; +int max_snaps = 128; +int snap_count, protected; +size_t i; +rbd_snap_info_t *snaps; +rbd_image_t image = NULL; + +r = rbd_open(ioctx, vol->name, &image, NULL); +if (r < 0) { + virReportSystemError(-r, _("failed to open the RBD image '%s'"), +vol->name); + goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) +VIR_FREE(snaps); + +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count, + source->name, vol->name); + +if (snap_count > 0) { +for (i = 0; i < snap_count; i++) { +if (rbd_snap_is_protected(image, snaps[i].name, &protected)) { +virReportSystemError(-r, _("failed to verify if snapshot '%s/%s@%s' is protected"), + source->name, vol->name, + snaps[i].name); +goto cleanup; +} + +if (protected == 1) { +VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be " + "unprotected", source->name, vol->name, + snaps[i].name); + +if (rbd_snap_unprotect(image, snaps[i].name) < 0) { +virReportSystemError(-r, _("failed to unprotect snapshot '%s/%s@%s'"), + source->name, vol->name, + snaps[i].name); +goto cleanup; +} +} + +VIR_DEBUG("Removing snapshot %s/%s@%s", source->name, + vol->name, snaps[i].name); + +r = rbd_snap_remove(image, snaps[i].name); +if (r < 0) { +virReportSystemError(-r, _("failed to remove snapshot '%s/%s@%s'"), + source->name, vol->name, + snaps[i].name); +goto cleanup; +} +} +} + +ret = 0; + + cleanup: +if (snaps) +rbd_snap_list_end(snaps); + +VIR_FREE(snaps); + +if (image) +rbd_close(image); + +return ret; +} + static int virStorageBackendRBDDeleteVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -443,6 +524,14 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) goto cleanup; +if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) { +if (virStorageBackendRBDCleanupSnapshots(ptr.ioctx, &pool->def->source, + vol) < 0) +goto cleanup; +} + +VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name); + r = rbd_remove(ptr.ioctx, vol->name); if (r < 0 && (-r) != ENOENT) { virReportSystemError(-r, _("failed to remove volume '%s/%s'"), -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Remove snapshots if the DELETE_WITH_SNAPSHOTS flag has been provided
When a RBD volume has snapshots it can not be removed. This patch introduces a new flag to force volume removal, VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS. With this flag any existing snapshots will be removed prior to removing the volume. No existing mechanism in libvirt allowed us to pass such information, so that's why a new flag was introduced. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 1 + src/storage/storage_backend_rbd.c | 91 +++ 2 files changed, 92 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 453089e..9fc3c2d 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -115,6 +115,7 @@ typedef enum { typedef enum { VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */ VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros (slow) */ +VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 1 << 1, /* Force removal of volume, even if in use */ } virStorageVolDeleteFlags; typedef enum { diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5ae4713..b494c11 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -421,6 +421,75 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, return ret; } +static int virStorageBackendRBDCleanupSnapshots(rbd_image_t image, +virStoragePoolSourcePtr source, +virStorageVolDefPtr vol) +{ +int ret = -1; +int r = 0; +int max_snaps = 128; +int snap_count, protected; +size_t i; +rbd_snap_info_t *snaps; + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) +VIR_FREE(snaps); + +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count, + source->name, vol->name); + +if (snap_count > 0) { +for (i = 0; i < snap_count; i++) { +if (rbd_snap_is_protected(image, snaps[i].name, &protected)) { +virReportSystemError(-r, _("failed to verify if snapshot '%s/%s@%s' is protected"), + source->name, vol->name, + snaps[i].name); +goto cleanup; +} + +if (protected == 1) { +VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be " + "unprotected", source->name, vol->name, + snaps[i].name); + +if (rbd_snap_unprotect(image, snaps[i].name) < 0) { +virReportSystemError(-r, _("failed to unprotect snapshot '%s/%s@%s'"), + source->name, vol->name, + snaps[i].name); +goto cleanup; +} +} + +VIR_DEBUG("Removing snapshot %s/%s@%s", source->name, + vol->name, snaps[i].name); + +r = rbd_snap_remove(image, snaps[i].name); +if (r < 0) { +virReportSystemError(-r, _("failed to remove snapshot '%s/%s@%s'"), + source->name, vol->name, + snaps[i].name); +goto cleanup; +} +} +} + +ret = 0; + + cleanup: +if (snaps) +rbd_snap_list_end(snaps); + +VIR_FREE(snaps); +return ret; +} + static int virStorageBackendRBDDeleteVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -431,6 +500,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; +rbd_image_t image = NULL; VIR_DEBUG("Removing RBD image %s/%s", pool->def->source.name, vol->name); @@ -443,6 +513,24 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) goto cleanup; +r = rbd_open(ptr.ioctx, vol->name, &image, NULL); +if (r < 0) { + virReportSystemError(-r, _("failed to open the RBD image '%s'"), +vol->name); + goto cleanup; +} + +if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) { +if (virStorageBackendRBDCleanupSnapshots(image, &pool->def->source, vol) < 0) +goto cleanup; +
[libvirt] [PATCH] rbd: Remove snapshots if the DELETE_WITH_SNAPSHOTS flag has been provided
When a RBD volume has snapshots it can not be removed. This patch introduces a new flag to force volume removal, VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS. With this flag any existing snapshots will be removed prior to removing the volume. No existing mechanism in libvirt allowed us to pass such information, so that's why a new flag was introduced. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 1 + src/storage/storage_backend_rbd.c | 59 +++ 2 files changed, 60 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 453089e..80da5a3 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -115,6 +115,7 @@ typedef enum { typedef enum { VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */ VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros (slow) */ +VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 2, /* Force removal of volume, even if in use */ } virStorageVolDeleteFlags; typedef enum { diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5ae4713..71f7d10 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -428,9 +428,13 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, { int ret = -1; int r = 0; +int max_snaps = 128; +int i, snap_count, protected; virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; +rbd_snap_info_t *snaps; +rbd_image_t image = NULL; VIR_DEBUG("Removing RBD image %s/%s", pool->def->source.name, vol->name); @@ -443,6 +447,60 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) goto cleanup; +r = rbd_open(ptr.ioctx, vol->name, &image, NULL); +if (r < 0) { + virReportSystemError(-r, _("failed to open the RBD image '%s'"), +vol->name); + goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) { +VIR_FREE(snaps); +} +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count, + pool->def->source.name, vol->name); + +if (snap_count > 0 && (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS)) { +for (i = 0; i < snap_count; i++) { +if (rbd_snap_is_protected(image, snaps[i].name, &protected)) +goto cleanup; + +if (protected == 1) { +VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be " + "unprotected", pool->def->source.name, vol->name, + snaps[i].name); + +if (rbd_snap_unprotect(image, snaps[i].name) < 0) +goto cleanup; +} + +VIR_DEBUG("Removing snapshot %s/%s@%s", pool->def->source.name, + vol->name, snaps[i].name); + +r = rbd_snap_remove(image, snaps[i].name); +if (r < 0) { +virReportSystemError(-r, _("failed to remove snapshot '%s/%s@%s'"), + pool->def->source.name, vol->name, + snaps[i].name); +goto cleanup; +} +} + +rbd_snap_list_end(snaps); +} + +if (rbd_close(image) < 0) +goto cleanup; + +VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name); + r = rbd_remove(ptr.ioctx, vol->name); if (r < 0 && (-r) != ENOENT) { virReportSystemError(-r, _("failed to remove volume '%s/%s'"), @@ -453,6 +511,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, ret = 0; cleanup: +VIR_FREE(snaps); virStorageBackendRBDCloseRADOSConn(&ptr); return ret; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Remove snapshots if the FORCED flag has been provided
On 15-10-15 10:44, Daniel P. Berrange wrote: > On Thu, Oct 15, 2015 at 10:28:39AM +0200, Wido den Hollander wrote: >> When a RBD volume has snapshots it can not be removed. >> >> This patch introduces a new flag to force volume removal, >> VIR_STORAGE_VOL_DELETE_FORCED. >> >> With this flag any existing snapshots will be removed prior >> to removing the volume. >> >> No existing mechanism in libvirt allowed us to pass such information, >> so that's why a new flag was introduced. >> >> Signed-off-by: Wido den Hollander >> --- >> include/libvirt/libvirt-storage.h | 1 + >> src/storage/storage_backend_rbd.c | 58 >> +++ >> 2 files changed, 59 insertions(+) >> >> diff --git a/include/libvirt/libvirt-storage.h >> b/include/libvirt/libvirt-storage.h >> index 453089e..36ff979 100644 >> --- a/include/libvirt/libvirt-storage.h >> +++ b/include/libvirt/libvirt-storage.h >> @@ -115,6 +115,7 @@ typedef enum { >> typedef enum { >> VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */ >> VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros >> (slow) */ >> +VIR_STORAGE_VOL_DELETE_FORCED = 2, /* Force removal of volume, even if >> in use */ > > Long term I could imagine there will be a number of reasons why > it might be forbidden to delete a volume by default. It would be > nice to selectively override these reasons. FORCED is quite a > generic name for a specific action. So how about naming it > VOL_DELETE_WITH_SNAPSHOTS > Seems like a good thing to me. Want me to submit a new patch? Before I do so, any code-wide objections? Wido > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Remove snapshots if the FORCED flag has been provided
When a RBD volume has snapshots it can not be removed. This patch introduces a new flag to force volume removal, VIR_STORAGE_VOL_DELETE_FORCED. With this flag any existing snapshots will be removed prior to removing the volume. No existing mechanism in libvirt allowed us to pass such information, so that's why a new flag was introduced. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 1 + src/storage/storage_backend_rbd.c | 58 +++ 2 files changed, 59 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 453089e..36ff979 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -115,6 +115,7 @@ typedef enum { typedef enum { VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */ VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros (slow) */ +VIR_STORAGE_VOL_DELETE_FORCED = 2, /* Force removal of volume, even if in use */ } virStorageVolDeleteFlags; typedef enum { diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index ac5085a..ca5e802 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -428,9 +428,13 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, { int ret = -1; int r = 0; +int max_snaps = 128; +int i, snap_count, protected; virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; +rbd_snap_info_t *snaps; +rbd_image_t image = NULL; VIR_DEBUG("Removing RBD image %s/%s", pool->def->source.name, vol->name); @@ -443,6 +447,59 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) goto cleanup; +r = rbd_open(ptr.ioctx, vol->name, &image, NULL); +if (r < 0) { + virReportSystemError(-r, _("failed to open the RBD image '%s'"), +vol->name); + goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) { +VIR_FREE(snaps); +} +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count, + pool->def->source.name, vol->name); + +if (snap_count > 0 && (flags & VIR_STORAGE_VOL_DELETE_FORCED)) { +for (i = 0; i < snap_count; i++) { +if (rbd_snap_is_protected(image, snaps[i].name, &protected)) +goto cleanup; + +if (protected == 1) { +VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be " + "unprotected", pool->def->source.name, vol->name, + snaps[i].name); + +if (rbd_snap_unprotect(image, snaps[i].name) < 0) +goto cleanup; +} + +VIR_DEBUG("Removing snapshot %s/%s@%s", pool->def->source.name, + vol->name, snaps[i].name); + +if (rbd_snap_remove(image, snaps[i].name) < 0) { +virReportSystemError(-r, _("failed to remove snapshot '%s/%s@%s'"), + pool->def->source.name, vol->name, + snaps[i].name); +goto cleanup; +} +} + +rbd_snap_list_end(snaps); +} + +if (rbd_close(image) < 0) +goto cleanup; + +VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name); + r = rbd_remove(ptr.ioctx, vol->name); if (r < 0 && (-r) != ENOENT) { virReportSystemError(-r, _("failed to remove volume '%s/%s'"), @@ -453,6 +510,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, ret = 0; cleanup: +VIR_FREE(snaps); virStorageBackendRBDCloseRADOSConn(&ptr); return ret; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Remove snapshots if the FORCED flag has been provided
When a RBD volume has snapshots it can not be removed. This patch introduces a new flag to force volume removal, VIR_STORAGE_VOL_DELETE_FORCED. With this flag any existing snapshots will be removed prior to removing the volume. No existing mechanism in libvirt allowed us to pass such information, so that's why a new flag was introduced. Signed-off-by: Wido den Hollander --- include/libvirt/libvirt-storage.h | 1 + src/storage/storage_backend_rbd.c | 58 +++ 2 files changed, 59 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 453089e..36ff979 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -115,6 +115,7 @@ typedef enum { typedef enum { VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */ VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros (slow) */ +VIR_STORAGE_VOL_DELETE_FORCED = 2, /* Force removal of volume, even if in use */ } virStorageVolDeleteFlags; typedef enum { diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index ac5085a..ca5e802 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -428,9 +428,13 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, { int ret = -1; int r = 0; +int max_snaps = 128; +int i, snap_count, protected; virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; +rbd_snap_info_t *snaps; +rbd_image_t image = NULL; VIR_DEBUG("Removing RBD image %s/%s", pool->def->source.name, vol->name); @@ -443,6 +447,59 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) goto cleanup; +r = rbd_open(ptr.ioctx, vol->name, &image, NULL); +if (r < 0) { + virReportSystemError(-r, _("failed to open the RBD image '%s'"), +vol->name); + goto cleanup; +} + +do { +if (VIR_ALLOC_N(snaps, max_snaps)) +goto cleanup; + +snap_count = rbd_snap_list(image, snaps, &max_snaps); +if (snap_count <= 0) { +VIR_FREE(snaps); +} +} while (snap_count == -ERANGE); + +VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count, + pool->def->source.name, vol->name); + +if (snap_count > 0 && (flags & VIR_STORAGE_VOL_DELETE_FORCED)) { +for (i = 0; i < snap_count; i++) { +if (rbd_snap_is_protected(image, snaps[i].name, &protected)) +goto cleanup; + +if (protected == 1) { +VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be " + "unprotected", pool->def->source.name, vol->name, + snaps[i].name); + +if (rbd_snap_unprotect(image, snaps[i].name) < 0) +goto cleanup; +} + +VIR_DEBUG("Removing snapshot %s/%s@%s", pool->def->source.name, + vol->name, snaps[i].name); + +if (rbd_snap_remove(image, snaps[i].name) < 0) { +virReportSystemError(-r, _("failed to remove snapshot '%s/%s@%s'"), + pool->def->source.name, vol->name, + snaps[i].name); +goto cleanup; +} +} + +rbd_snap_list_end(snaps); +} + +if (rbd_close(image) < 0) +goto cleanup; + +VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name); + r = rbd_remove(ptr.ioctx, vol->name); if (r < 0 && (-r) != ENOENT) { virReportSystemError(-r, _("failed to remove volume '%s/%s'"), @@ -453,6 +510,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, ret = 0; cleanup: +VIR_FREE(snaps); virStorageBackendRBDCloseRADOSConn(&ptr); return ret; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java] Require at least Java 8
On 04-09-15 07:12, Claudio Bley wrote: > At Wed, 2 Sep 2015 17:38:03 +0200, > Wido den Hollander wrote: >> >> >> >>> The current code doesn't compile under Java 7, but Java 7 is also EOL. >>> >>> >>> That should not be the case. Can you provide the error messages or >>> simply point to the place where Java 8 classes are used? >>> >> >> build: >> [javac] Compiling 98 source files to >> /home/wido/repos/libvirt-java/target/classes >> [javac] warning: [options] bootstrap class path not set in >> conjunction with -source 1.7 >> [javac] >> /home/wido/repos/libvirt-java/src/main/java/org/libvirt/event/DomainEvent.java:63: >> error: incompatible types: inference variable T#1 has incompatible upper >> bounds Enum,T#3 >> [javac] return this.type.obtain(this.detail); >> [javac]^ >> [javac] where T#1,T#2,T#3 are type-variables: >> [javac] T#1 extends Enum declared in method obtain(int) >> [javac] T#2 extends T#3 >> [javac] T#3 extends Enum,DomainEventDetail declared in >> method getDetail() >> [javac] 1 error >> [javac] 1 warning > > This reminded me of a problem I once saw with the Java 7 (I think) > compiler being unable to correctly infer the types when calling static > generic methods. > > Since I always used the Java 7 compiler to compile the code and as I > saw the javac warning above, I got curious. > > The thing is: the code compiles just fine with an actual Java 7 > compiler (OpenJDK 1.7.0_85), but not when using a Java 8 compiler > (OpenJDK 1.8.0_60) with the `-source 1.7` switch. > Ah, indeed. I'm using the Oracle Java 8 JDK and not OpenJDK. >> But if the compile issue can be fixed we can probably require at least >> Java 7. I think Java 6 is dangerous. > > I'm still thinking about this. Maybe you're right. But still, the code > is valid 1.7 source code. Should we prevent that code from compiling > on Java 7 to protect users from using an obsolete JVM? I don't think > this is the right place to control that. > No, we can probably depend on Java 7, but we should update the README that it doesn't compile properly on Oracle's JDK and you should use OpenJDK. Wido > -- > Claudio > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java] Require at least Java 8
On 02-09-15 14:57, Claudio Bley wrote: > On Wed, Sep 2, 2015 at 1:53 PM, Wido den Hollander <mailto:w...@widodh.nl>> wrote: > > > > > I know nothing about libvirt-java, but maybe you can be more > verbose why > > this change is needed. > > > > The current code doesn't compile under Java 7, but Java 7 is also EOL. > > > That should not be the case. Can you provide the error messages or > simply point to the place where Java 8 classes are used? > build: [javac] Compiling 98 source files to /home/wido/repos/libvirt-java/target/classes [javac] warning: [options] bootstrap class path not set in conjunction with -source 1.7 [javac] /home/wido/repos/libvirt-java/src/main/java/org/libvirt/event/DomainEvent.java:63: error: incompatible types: inference variable T#1 has incompatible upper bounds Enum,T#3 [javac] return this.type.obtain(this.detail); [javac]^ [javac] where T#1,T#2,T#3 are type-variables: [javac] T#1 extends Enum declared in method obtain(int) [javac] T#2 extends T#3 [javac] T#3 extends Enum,DomainEventDetail declared in method getDetail() [javac] 1 error [javac] 1 warning > Java 8 is the way forward since there won't be any security fixes for > Java 7 anymore. > > > Yes, but maybe there're some distributions / organizations which still > depend on Java 7 or 6. I don't want to raise the bar without good reason. > Ok, fair enough. I think that Java 8 is fine since that is available in the major distributions. But if the compile issue can be fixed we can probably require at least Java 7. I think Java 6 is dangerous. Wido > -- > Claudio > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java] Require at least Java 8
On 02-09-15 13:11, Michal Privoznik wrote: > On 28.08.2015 13:12, Wido den Hollander wrote: >> Signed-off-by: Wido den Hollander >> --- >> INSTALL | 2 +- >> build.properties | 6 +++--- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/INSTALL b/INSTALL >> index 581512d..2cdd829 100644 >> --- a/INSTALL >> +++ b/INSTALL >> @@ -11,7 +11,7 @@ main item you may need to change in this file is the >> jars.dir >> property. This property should point to a directory which contains >> the junit.jar and jna.jar files. >> >> -You will need a Java Development Kit accepting the version 1.6 >> +You will need a Java Development Kit accepting the version 1.8 >> of the language since the bindings use enums as well as the new >> for loop syntax >> >> diff --git a/build.properties b/build.properties >> index bcc7b8c..c7c1a43 100644 >> --- a/build.properties >> +++ b/build.properties >> @@ -1,8 +1,8 @@ >> version=0.5.1 >> release=1 >> libvirt.required=0.9.12 >> -java.required=1.6.0 >> -java.target=1.6 >> -java.source=1.6 >> +java.required=1.8.0 >> +java.target=1.8 >> +java.source=1.8 >> rpm.topdir=/home/veillard/rpms >> jar.dir=/usr/share/java >> > > I know nothing about libvirt-java, but maybe you can be more verbose why > this change is needed. > The current code doesn't compile under Java 7, but Java 7 is also EOL. Java 8 is the way forward since there won't be any security fixes for Java 7 anymore. Wido > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] (no subject)
The current code of libvirt-java does not build with Java 7. This patch updates the build.properties and INSTALL that we require Java 8 to build libvirt-java. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-java] Require at least Java 8
Signed-off-by: Wido den Hollander --- INSTALL | 2 +- build.properties | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/INSTALL b/INSTALL index 581512d..2cdd829 100644 --- a/INSTALL +++ b/INSTALL @@ -11,7 +11,7 @@ main item you may need to change in this file is the jars.dir property. This property should point to a directory which contains the junit.jar and jna.jar files. -You will need a Java Development Kit accepting the version 1.6 +You will need a Java Development Kit accepting the version 1.8 of the language since the bindings use enums as well as the new for loop syntax diff --git a/build.properties b/build.properties index bcc7b8c..c7c1a43 100644 --- a/build.properties +++ b/build.properties @@ -1,8 +1,8 @@ version=0.5.1 release=1 libvirt.required=0.9.12 -java.required=1.6.0 -java.target=1.6 -java.source=1.6 +java.required=1.8.0 +java.target=1.8 +java.source=1.8 rpm.topdir=/home/veillard/rpms jar.dir=/usr/share/java -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use RBD format 2 by default when creating images.
On 16-07-15 18:28, John Ferlan wrote: > > > On 07/14/2015 04:13 PM, Josh Durgin wrote: >> On 07/14/2015 12:42 PM, John Ferlan wrote: >>> >>> >>> On 07/14/2015 04:15 AM, Wido den Hollander wrote: >>>> We used to look at the librbd code version and depending on >>>> that we would invoke rbd_create3() or rbd_create(). >>>> >>>> Since librbd version 0.67.9 we can however tell RBD that it >>>> should create rbd format 2 images even if we invoke >>>> rbd_create(). >>>> >>>> The>> less options we pass to librbd, the more we can lean on >>>> the sane defaults it uses. >>>> >>>> For rbd_create3() we had things like the stripe count and >>>> unit hardcoded in libvirt and that might cause problems down >>>> the road. >> >> Hardcoding the feature bits is even worse. I think this is the >> right approach. >> > > OK - just trying to be sure... and since no one else spoke up > yet, seems the approach is fine. > Yes, I think Josh gave the right comments. > ... > >>>> virStorageBackendRBDCreateImage(rados_ioctx_t io, char *name, >>>> long capacity) { int order = 0; -#if LIBRBD_VERSION_CODE > >>>> 260 -uint64_t features = 3; -uint64_t stripe_count = >>>> 1; -uint64_t stripe_unit = 4194304; - -if >>>> (rbd_create3(io, name, capacity, features, &order, - >>>> stripe_unit, stripe_count) < 0) { -#else if (rbd_create(io, >>>> name, capacity, &order) < 0) { -#endif > > This change fails make syntax-check due to unnecessary curly > braces. > > Before I push, the question I see a second issue... > > The current code returns -1 regardless of error, which is then used > in a call to virReportSystemError(-r,...)... That means regardless > of the error you're returning EPERM which could throw someone off > if something other than PERM was returned from rbd_create > > I can fix that as well by : > > return rbd_create(io, name, capacity, &order); > Seems like a sane approach to me. Less if-statements and the code still works just fine. You can apply my patch and fix this afterwards? Or do you want a new patch from me? Wido > > FWIW: This reporting snafu was introduced by commit id '761491e' > which just took the return of virStorageBackendRBDCreateImage and > used it as the basis for the message generated. > > > John > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Use RBD format 2 by default when creating images.
We used to look at the librbd code version and depending on that we would invoke rbd_create3() or rbd_create(). Since librbd version 0.67.9 we can however tell RBD that it should create rbd format 2 images even if we invoke rbd_create(). The less options we pass to librbd, the more we can lean on the sane defaults it uses. For rbd_create3() we had things like the stripe count and unit hardcoded in libvirt and that might cause problems down the road. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8e8d7a7..936ad18 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -66,6 +66,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, const char *client_mount_timeout = "30"; const char *mon_op_timeout = "30"; const char *osd_op_timeout = "30"; +const char *rbd_default_format = "2"; if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); @@ -211,6 +212,14 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, VIR_DEBUG("Setting RADOS option rados_osd_op_timeout to %s", osd_op_timeout); rados_conf_set(ptr->cluster, "rados_osd_op_timeout", osd_op_timeout); +/* + * Librbd supports creating RBD format 2 images. We no longer have to invoke + * rbd_create3(), we can tell librbd to default to format 2. + * This leaves us to simply use rbd_create() and use the default behavior of librbd + */ +VIR_DEBUG("Setting RADOS option rbd_default_format to %s", rbd_default_format); +rados_conf_set(ptr->cluster, "rbd_default_format", rbd_default_format); + ptr->starttime = time(0); r = rados_connect(ptr->cluster); if (r < 0) { @@ -475,16 +484,7 @@ static int virStorageBackendRBDCreateImage(rados_ioctx_t io, char *name, long capacity) { int order = 0; -#if LIBRBD_VERSION_CODE > 260 -uint64_t features = 3; -uint64_t stripe_count = 1; -uint64_t stripe_unit = 4194304; - -if (rbd_create3(io, name, capacity, features, &order, -stripe_unit, stripe_count) < 0) { -#else if (rbd_create(io, name, capacity, &order) < 0) { -#endif return -1; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] storage conf: Add key-value options to storage pools
On 06/04/2014 12:14 PM, Daniel P. Berrange wrote: On Tue, Jun 03, 2014 at 05:33:13PM +0200, Ján Tomko wrote: On 05/28/2014 11:38 AM, Wido den Hollander wrote: This series of patches adds the ability to pass down options to the storage pool drivers. In the case of NFS users can specify mount options and in the case of RBD users can specify options for librados to influence some behavior. All options and values are checked on input validity to prevent injection of malicious commands. Should we allow arbitrary options? No, we shouldn't - at least not in this way. Libvirt's goal is always to to provide a clearly defined mapping, not do arbitrary unchecked argument passthrough. History has shown repeatedly that tools/libraries change their syntax and libvirt has demonstrated its value in adapting to these changes without breaking application compatibility. Ok, understood. I didn't expect these patches to make it, but I wanted to start the discussion. The only way I'd support passthrough is if it were done in he same way as QEMU passthrough where it used a separate XML namespace which was clearly marked "use at your own risk, unsupported if it breaks". Well, that would be something which is useful. For Ceph there are a lot of options, but NFS users might want to specify "noatime" or w and rsize as mount options. Wido Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] storage conf: Support setting RADOS options in RBD storage backend
From: Wido den Hollander This way users can manually set options in librados which might suite their needs better. --- docs/schemas/storagepool.rng |1 + src/storage/storage_backend_rbd.c| 16 tests/storagepoolxml2xmlin/pool-rbd.xml |3 +++ tests/storagepoolxml2xmlout/pool-rbd.xml |3 +++ 4 files changed, 23 insertions(+) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 7b38dce..0cd119c 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -568,6 +568,7 @@ + diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 5d4ef79..3cb5b85 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -216,6 +216,22 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, VIR_DEBUG("Setting RADOS option rados_osd_op_timeout to %s", osd_op_timeout); rados_conf_set(ptr->cluster, "rados_osd_op_timeout", osd_op_timeout); +if (pool->def->source.noptions > 0) { +for (i = 0; i < pool->def->source.noptions; i++) { +if (pool->def->source.options[i].name != NULL && +pool->def->source.options[i].value != NULL) { +VIR_DEBUG("Setting RADOS option %s to %s", +pool->def->source.options[i].name, +pool->def->source.options[i].value); +if (rados_conf_set(ptr->cluster, pool->def->source.options[i].name, +pool->def->source.options[i].value) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set RADOS option %s"), + pool->def->source.options[i].name); +} +} +} +} + ptr->starttime = time(0); r = rados_connect(ptr->cluster); if (r < 0) { diff --git a/tests/storagepoolxml2xmlin/pool-rbd.xml b/tests/storagepoolxml2xmlin/pool-rbd.xml index 056ba6e..05991fc 100644 --- a/tests/storagepoolxml2xmlin/pool-rbd.xml +++ b/tests/storagepoolxml2xmlin/pool-rbd.xml @@ -7,5 +7,8 @@ + + + diff --git a/tests/storagepoolxml2xmlout/pool-rbd.xml b/tests/storagepoolxml2xmlout/pool-rbd.xml index 4fe2fce..cb9969e 100644 --- a/tests/storagepoolxml2xmlout/pool-rbd.xml +++ b/tests/storagepoolxml2xmlout/pool-rbd.xml @@ -11,5 +11,8 @@ + + + -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] storage conf: Add key-value options to storage pools
This series of patches adds the ability to pass down options to the storage pool drivers. In the case of NFS users can specify mount options and in the case of RBD users can specify options for librados to influence some behavior. All options and values are checked on input validity to prevent injection of malicious commands. -- Wido den Hollander -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] storage conf: Support mount options for NetFS pools
This way users can provide mount options for for example NFS storage pools. --- src/storage/storage_backend_fs.c | 44 ++ 1 file changed, 44 insertions(+) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 33551e7..66d7bec 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -377,6 +377,8 @@ static int virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { char *src = NULL; +char *options = NULL; +char *optionsflag = NULL; /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), * while plain 'mount' does. We have to craft separate argvs to * accommodate this */ @@ -385,8 +387,10 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) bool glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); virCommandPtr cmd = NULL; +virBuffer optionsbuf = VIR_BUFFER_INITIALIZER; int ret = -1; int rc; +int i; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { if (pool->def->source.nhost != 1) { @@ -432,9 +436,45 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) if (VIR_STRDUP(src, pool->def->source.devices[0].path) < 0) return -1; } + +/* + * Mount options for NFS pool. + * For security reasons we do not simply build a string based on + * the given mount options. This is to prevent any shell injection + * or non-valid mount options. + */ +if (pool->def->type == VIR_STORAGE_POOL_NETFS) { +if (pool->def->source.noptions > 0) { +for (i = 0; i < pool->def->source.noptions; i++) { +char *name = pool->def->source.options[i].name; +char *value = pool->def->source.options[i].value; +if (name != NULL && value == NULL) +virBufferAsprintf(&optionsbuf, "%s,", name); + +if (name != NULL && value != NULL) +virBufferAsprintf(&optionsbuf, "%s=%s,", name, value); +} + +if (virBufferError(&optionsbuf)) +goto no_memory; + +/* + * Strip the last character from the options string since + * that will be a comma. + */ +options = virBufferContentAndReset(&optionsbuf); +if (options != NULL) { +options[strlen(options)-1] = 0; +if (virAsprintf(&optionsflag, "%s", "-o") == -1) +return -1; +} +} +} if (netauto) cmd = virCommandNewArgList(MOUNT, + optionsflag, + options, src, pool->def->target.path, NULL); @@ -455,6 +495,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) (pool->def->type == VIR_STORAGE_POOL_FS ? virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format)), + optionsflag, + options, src, pool->def->target.path, NULL); @@ -463,6 +505,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) goto cleanup; ret = 0; + no_memory: +virReportOOMError(); cleanup: virCommandFree(cmd); VIR_FREE(src); -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] storage conf: Add support for key-value pair options for pools
From: Wido den Hollander This allows the end-user to pass down options to the storage pool backend. For example NFS could get mount options or Ceph librados options passed down. --- docs/schemas/storagepool.rng | 16 + docs/storage.html.in | 48 + src/conf/storage_conf.c | 82 +- src/conf/storage_conf.h | 15 4 files changed, 160 insertions(+), 1 deletion(-) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 8d7a94d..7b38dce 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -294,6 +294,22 @@ + + + + + + + + + + + + + + + + diff --git a/docs/storage.html.in b/docs/storage.html.in index eb38b16..3acbd53 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -270,6 +270,30 @@ </target> </pool> +Mount options + + It is also possible to influence the mount options used for NFS by + adding extra options in the pool's XML definition. + + + <pool type="netfs"> +<name>virtimages</name> +<source> + <host name="nfs.example.com"/> + <dir path="/var/lib/virt/images"/> + <format type='nfs'/> + <option name='noatime'/> + <option name='rsize' value='8k'/> +</source> +<target> + <path>/var/lib/virt/images</path> +</target> + </pool> + + Storage pool options are support since 1.2.6 + + + Valid pool format types The network filesystem pool supports the following formats: @@ -561,6 +585,30 @@ </source> </pool> +RADOS cluster options + + It is also possible to influence the RADOS cluster options used by librados + by adding extra options in the pool's XML definition. + + + <pool type="rbd"> +<name>myrbdpool</name> +<source> + <name>rbdpool</name> +<host name='1.2.3.4' port='6789'/> +<host name='my.ceph.monitor' port='6789'/> +<host name='third.ceph.monitor' port='6789'/> +<auth username='admin' type='ceph'> + <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> +</auth> + <option name='rados_mon_op_timeout' value='10'/> + <option name='rados_osd_op_timeout' value='10'/> +</source> + </pool> + + Storage pool options are support since 1.2.6 + + Example volume output <volume> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8b6fd79..777e12d 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -32,6 +32,7 @@ #include #include #include +#include #include "virerror.h" #include "datatypes.h" @@ -370,6 +371,11 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source) for (i = 0; i < source->ndevice; i++) virStoragePoolSourceDeviceClear(&source->devices[i]); VIR_FREE(source->devices); +for (i = 0; i < source->noptions; i++) { +VIR_FREE(source->options[i].name); +VIR_FREE(source->options[i].value); +} +VIR_FREE(source->options); VIR_FREE(source->dir); VIR_FREE(source->name); virStoragePoolSourceAdapterClear(source->adapter); @@ -551,6 +557,32 @@ virStoragePoolDefParseAuth(xmlXPathContextPtr ctxt, return ret; } +static int virStoragePoolDefVerifyOption(char *msg) +{ +regex_t regex; +int reti; +char msgbuf[100]; +int ret = -1; + +reti = regcomp(®ex, "^[A-Za-z0-9][A-Za-z0-9_-]*[A-Za-z0-9]$", 0); +if (reti) +goto cleanup; + +reti = regexec(®ex, msg, 0, NULL, 0); +if (!reti) { +ret = 0; +} else if (reti == REG_NOMATCH) { +ret = -1; +} else { +regerror(reti, ®ex, msgbuf, sizeof(msgbuf)); +ret = -2; +} + +cleanup: +regfree(®ex); +return ret; +} + static int virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, virStoragePoolSourcePtr source, @@ -559,7 +591,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, { int ret = -1; xmlNodePtr relnode, *nodeset = NULL; -int nsource; +int nsource, noptions; size_t i; virStoragePoolOptionsPtr options; char
[libvirt] [PATCH 2/2] rbd: Set timeout options for librados
These timeout values make librados/librbd return -ETIMEDOUT when a operation is blocking due to a failing/unreachable Ceph cluster. By having the operations time out libvirt will not block. --- src/storage/storage_backend_rbd.c | 17 + 1 file changed, 17 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index abb110e..22ed7de 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -60,6 +60,9 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, char secretUuid[VIR_UUID_STRING_BUFLEN]; size_t i; char *mon_buff = NULL; +const char *client_mount_timeout = "30"; +const char *mon_op_timeout = "30"; +const char *osd_op_timeout = "30"; VIR_DEBUG("Found Cephx username: %s", pool->def->source.auth.cephx.username); @@ -197,6 +200,20 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, goto cleanup; } +/* + * Set timeout options for librados. + * In case the Ceph cluster is down libvirt won't block forever. + * Operations in librados will return -ETIMEDOUT when the timeout is reached. + */ +VIR_DEBUG("Setting RADOS option client_mount_timeout to %s", client_mount_timeout); +rados_conf_set(ptr->cluster, "client_mount_timeout", client_mount_timeout); + +VIR_DEBUG("Setting RADOS option rados_mon_op_timeout to %s", mon_op_timeout); +rados_conf_set(ptr->cluster, "rados_mon_op_timeout", mon_op_timeout); + +VIR_DEBUG("Setting RADOS option rados_osd_op_timeout to %s", osd_op_timeout); +rados_conf_set(ptr->cluster, "rados_osd_op_timeout", osd_op_timeout); + ptr->starttime = time(0); r = rados_connect(ptr->cluster); if (r < 0) { -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] rbd: Include return statusses from librados/librbd in logging
With this information it's easier for the user to debug what is going wrong. --- src/storage/storage_backend_rbd.c | 127 ++--- 1 file changed, 61 insertions(+), 66 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 1b35cc4..abb110e 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -51,6 +51,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) { int ret = -1; +int r = 0; unsigned char *secret_value = NULL; size_t secret_value_size; char *rados_key = NULL; @@ -65,10 +66,9 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, if (pool->def->source.auth.cephx.username != NULL) { VIR_DEBUG("Using cephx authorization"); -if (rados_create(&ptr->cluster, -pool->def->source.auth.cephx.username) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to initialize RADOS")); +r = rados_create(&ptr->cluster, pool->def->source.auth.cephx.username); +if (r < 0) { +virReportSystemError(-r, "%s", _("failed to initialize RADOS")); goto cleanup; } @@ -198,10 +198,10 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, } ptr->starttime = time(0); -if (rados_connect(ptr->cluster) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to connect to the RADOS monitor on: %s"), - mon_buff); +r = rados_connect(ptr->cluster); +if (r < 0) { +virReportSystemError(-r, _("failed to connect to the RADOS monitor on: %s"), + mon_buff); goto cleanup; } @@ -219,6 +219,16 @@ cleanup: return ret; } +static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) +{ +int r = rados_ioctx_create(ptr->cluster, pool->def->source.name, &ptr->ioctx); +if (r < 0) { +virReportSystemError(-r, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), + pool->def->source.name); +} +return r; +} + static int virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) { int ret = 0; @@ -248,18 +258,21 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStorageBackendRBDStatePtr ptr) { int ret = -1; +int r = 0; rbd_image_t image; -if (rbd_open(ptr->ioctx, vol->name, &image, NULL) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to open the RBD image '%s'"), - vol->name); + +r = rbd_open(ptr->ioctx, vol->name, &image, NULL); +if (r < 0) { +virReportSystemError(-r, _("failed to open the RBD image '%s'"), + vol->name); return ret; } rbd_image_info_t info; -if (rbd_stat(image, &info, sizeof(info)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to stat the RBD image")); +r = rbd_stat(image, &info, sizeof(info)); +if (r < 0) { +virReportSystemError(-r, _("failed to stat the RBD image '%s'"), + vol->name); goto cleanup; } @@ -297,6 +310,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, size_t max_size = 1024; int ret = -1; int len = -1; +int r = 0; char *name, *names = NULL; virStorageBackendRBDState ptr; ptr.cluster = NULL; @@ -306,26 +320,22 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, goto cleanup; } -if (rados_ioctx_create(ptr.cluster, -pool->def->source.name, &ptr.ioctx) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), - pool->def->source.name); +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) { goto cleanup; } struct rados_cluster_stat_t clusterstat; -if (rados_cluster_stat(ptr.cluster, &clusterstat) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to stat the RADOS cluster")); +r = rados_cluster_stat(ptr.cluster, &clusterstat); +if (r < 0) { +virReportSystemError(-r, "%s", _("failed to stat the RADOS cluster")); goto cleanup; } struct rados_pool_stat_t poolstat; -if (rados_ioctx_pool_stat(ptr.ioctx, &poolstat) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to stat the RADOS pool '%s'"), - pool->def->source.name); +r = rados_ioctx_pool_sta
[libvirt] [PATCH 0/2] Improve RBD storage pool backend
The first patch improves the logging for the user. Currently the return codes from librados are not written to any logfile, which might leave a user clueless to why the storage pool is not working. The second patch sets three timeout options in librados. These are useful for when the Ceph cluster is down. Librados will then not block for ever, but return a -ETIMEDOUT so libvirt does not block for ever. Wido den Hollander (2): rbd: Include return statusses from librados/librbd in logging rbd: Set timeout options for librados src/storage/storage_backend_rbd.c | 144 - 1 file changed, 78 insertions(+), 66 deletions(-) -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] rbd: Simplify opening RADOS IoCTX
Reduces code and brings logging back to one function. --- src/storage/storage_backend_rbd.c | 43 ++--- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index bd21873..8d1e320 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -221,6 +221,17 @@ cleanup: return ret; } +static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) +{ +int r = rados_ioctx_create(ptr->cluster, pool->def->source.name, &ptr->ioctx); +if (r < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), + pool->def->source.name, r); +} +return r; +} + static int virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr) { int ret = 0; @@ -313,11 +324,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, goto cleanup; } -r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx); -if (r < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), - pool->def->source.name, r); +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) { goto cleanup; } @@ -422,11 +429,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, goto cleanup; } -r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx); -if (r < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), - pool->def->source.name, r); +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) { goto cleanup; } @@ -510,13 +513,8 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0) goto cleanup; -r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx); -if (r < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), - pool->def->source.name, r); +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) goto cleanup; -} if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -551,17 +549,12 @@ static int virStorageBackendRBDRefreshVol(virConnectPtr conn, ptr.cluster = NULL; ptr.ioctx = NULL; int ret = -1; -int r = 0; if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0) { goto cleanup; } -r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx); -if (r < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), - pool->def->source.name, r); +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) { goto cleanup; } @@ -595,11 +588,7 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } -r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx); -if (r < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), - pool->def->source.name, r); +if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) { goto cleanup; } -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Various RBD storage pool changes
This series of patches changes a couple of things in the RBD storage pool driver. The first change is that it includes the return status of librados and librbd in log and debug messages making it easier for users to track down the real cause of the problem. The second patch simplifies some code. The third patch leverages the new timeout options from librados. Should the backing Ceph cluster not respond for any reason librados will timeout. This prevents libvirt from handing on a rados_* or rbd_* call and locking up libvirt. If the librados version on the system does not support these timeout options nothing happens. libvirt compiles and runs just fine, but the timeout simply won't work. Wido den Hollander (3): rbd: Include return statusses from librados/librbd in logging rbd: Simplify opening RADOS IoCTX rbd: Set timeout options for librados src/storage/storage_backend_rbd.c | 137 ++--- 1 file changed, 80 insertions(+), 57 deletions(-) -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] rbd: Include return statusses from librados/librbd in logging
With this information it's easier for the user to debug what is going wrong. --- src/storage/storage_backend_rbd.c | 119 + 1 file changed, 68 insertions(+), 51 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 1b35cc4..bd21873 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -51,6 +51,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) { int ret = -1; +int r = 0; unsigned char *secret_value = NULL; size_t secret_value_size; char *rados_key = NULL; @@ -65,10 +66,10 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, if (pool->def->source.auth.cephx.username != NULL) { VIR_DEBUG("Using cephx authorization"); -if (rados_create(&ptr->cluster, -pool->def->source.auth.cephx.username) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to initialize RADOS")); +r = rados_create(&ptr->cluster, pool->def->source.auth.cephx.username); +if (r < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to initialize RADOS: %d"), r); goto cleanup; } @@ -198,10 +199,11 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, } ptr->starttime = time(0); -if (rados_connect(ptr->cluster) < 0) { +r = rados_connect(ptr->cluster); +if (r < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to connect to the RADOS monitor on: %s"), - mon_buff); + _("failed to connect to the RADOS monitor on: %s: %d"), + mon_buff, r); goto cleanup; } @@ -248,18 +250,22 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStorageBackendRBDStatePtr ptr) { int ret = -1; +int r = 0; rbd_image_t image; -if (rbd_open(ptr->ioctx, vol->name, &image, NULL) < 0) { + +r = rbd_open(ptr->ioctx, vol->name, &image, NULL); +if (r < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to open the RBD image '%s'"), - vol->name); + _("failed to open the RBD image '%s': %d"), + vol->name, r); return ret; } rbd_image_info_t info; -if (rbd_stat(image, &info, sizeof(info)) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to stat the RBD image")); +r = rbd_stat(image, &info, sizeof(info)); +if (r < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to stat the RBD image: %d"), r); goto cleanup; } @@ -297,6 +303,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, size_t max_size = 1024; int ret = -1; int len = -1; +int r = 0; char *name, *names = NULL; virStorageBackendRBDState ptr; ptr.cluster = NULL; @@ -306,26 +313,28 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn, goto cleanup; } -if (rados_ioctx_create(ptr.cluster, -pool->def->source.name, &ptr.ioctx) < 0) { +r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx); +if (r < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), - pool->def->source.name); + _("failed to create the RBD IoCTX. Does the pool '%s' exist?: %d"), + pool->def->source.name, r); goto cleanup; } struct rados_cluster_stat_t clusterstat; -if (rados_cluster_stat(ptr.cluster, &clusterstat) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to stat the RADOS cluster")); +r = rados_cluster_stat(ptr.cluster, &clusterstat); +if (r < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to stat the RADOS cluster: %d"), r); goto cleanup; } struct rados_pool_stat_t poolstat; -if (rados_ioctx_pool_stat(ptr.ioctx, &poolstat) < 0) { +r = rados_ioctx_pool_stat(ptr.ioctx, &poolstat); +if (r < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to stat the RADOS pool '%s'"), - pool->def->source.name); + _("failed to stat the RADOS pool '%s': %d"), + pool->def->source.name, r); goto cleanup; } @@ -398,6 +407,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
[libvirt] [PATCH 3/3] rbd: Set timeout options for librados
These timeout values make librados/librbd return -ETIMEDOUT when a operation is blocking due to a failing/unreachable Ceph cluster. By having the operations time out libvirt will not block. --- src/storage/storage_backend_rbd.c | 17 + 1 file changed, 17 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8d1e320..2c97bf4 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -60,6 +60,9 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, char secretUuid[VIR_UUID_STRING_BUFLEN]; size_t i; char *mon_buff = NULL; +const char *client_mount_timeout = "30"; +const char *mon_op_timeout = "30"; +const char *osd_op_timeout = "30"; VIR_DEBUG("Found Cephx username: %s", pool->def->source.auth.cephx.username); @@ -198,6 +201,20 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, goto cleanup; } +/* + * Set timeout options for librados. + * In case the Ceph cluster is down libvirt won't block forever. + * Operations in librados will return -ETIMEDOUT when the timeout is reached. + */ +VIR_DEBUG("Setting RADOS option client_mount_timeout to %s", client_mount_timeout); +rados_conf_set(ptr->cluster, "client_mount_timeout", client_mount_timeout); + +VIR_DEBUG("Setting RADOS option rados_mon_op_timeout to %s", mon_op_timeout); +rados_conf_set(ptr->cluster, "rados_mon_op_timeout", mon_op_timeout); + +VIR_DEBUG("Setting RADOS option rados_osd_op_timeout to %s", osd_op_timeout); +rados_conf_set(ptr->cluster, "rados_osd_op_timeout", osd_op_timeout); + ptr->starttime = time(0); r = rados_connect(ptr->cluster); if (r < 0) { -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
On 01/30/2014 02:02 PM, Ján Tomko wrote: On 12/11/2013 03:47 PM, Wido den Hollander wrote: This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. Older versions of libvirt can work with this new RBD format as long as librbd supports format 2, something that all recent versions of librbd do. How recent? It might be nicer to mention the version number. Also, the patch no longer applies. I sent a revised version of the patch to the list last week. The commit message now shows the librbd versions required and it also applies to master again. Could you take a look at it again? It would really help the Ceph project. Thanks a lot! Wido Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 4b6f18c..f3dd7a0 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -435,6 +435,26 @@ cleanup: return ret; } +static int virStorageBackendRBDCreateImage(rados_ioctx_t io, + char *name, long capacity) +{ +int order = 0; +#if LIBRBD_VERSION_CODE > 260 This will fail 'make syntax-check' as it's not indented properly, see: http://libvirt.org/hacking.html#preprocessor It would also be easier to read if compared against LIBRBD_VERSION(0, 1, x), instead of 260. +uint64_t features = 3; +uint64_t stripe_count = 1; +uint64_t stripe_unit = 4194304; + Can these numbers be represented by more descriptive constants from librbd header files? Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit from the new RBD format. Older versions of libvirt can work with this new RBD format as long as librbd supports format 2. RBD format is supported by librbd since version 0.56 (Ceph Bobtail). Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index c5f0bc5..91c07ac 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -458,6 +458,25 @@ virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int virStorageBackendRBDCreateImage(rados_ioctx_t io, + char *name, long capacity) +{ +int order = 0; +#if LIBRBD_VERSION_CODE > 260 +uint64_t features = 3; +uint64_t stripe_count = 1; +uint64_t stripe_unit = 4194304; + +if (rbd_create3(io, name, capacity, features, &order, +stripe_count, stripe_unit) < 0) { +#else +if (rbd_create(io, name, capacity, &order) < 0) { +#endif +return -1; +} + +return 0; +} static int virStorageBackendRBDBuildVol(virConnectPtr conn, @@ -468,7 +487,6 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; -int order = 0; int ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", @@ -494,7 +512,7 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, goto cleanup; } -if (rbd_create(ptr.ioctx, vol->name, vol->capacity, &order) < 0) { +if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to create volume '%s/%s'"), pool->def->source.name, -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
On 12/11/2013 03:47 PM, Wido den Hollander wrote: This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. Older versions of libvirt can work with this new RBD format as long as librbd supports format 2, something that all recent versions of librbd do. Could somebody take a look at this patch? We would really like to see this in libvirt so it can make it's way into the Linux distributions. Wido Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 4b6f18c..f3dd7a0 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -435,6 +435,26 @@ cleanup: return ret; } +static int virStorageBackendRBDCreateImage(rados_ioctx_t io, + char *name, long capacity) +{ +int order = 0; +#if LIBRBD_VERSION_CODE > 260 +uint64_t features = 3; +uint64_t stripe_count = 1; +uint64_t stripe_unit = 4194304; + +if (rbd_create3(io, name, capacity, features, &order, +stripe_count, stripe_unit) < 0) { +#else +if (rbd_create(io, name, capacity, &order) < 0) { +#endif +return -1; +} + +return 0; +} + static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) @@ -442,7 +462,6 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; -int order = 0; int ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", @@ -467,7 +486,7 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, goto cleanup; } -if (rbd_create(ptr.ioctx, vol->name, vol->capacity, &order) < 0) { +if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to create volume '%s/%s'"), pool->def->source.name, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
On 01/02/2014 08:42 PM, Doug Goldstein wrote: On Thu, Jan 2, 2014 at 10:01 AM, Wido den Hollander wrote: On 12/11/2013 03:47 PM, Wido den Hollander wrote: This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. Older versions of libvirt can work with this new RBD format as long as librbd supports format 2, something that all recent versions of librbd do. Did somebody get a look at this patch yet? It's a fairly simple one, but would be great for the Ceph project! So I'm thinking we might want to do something similar with the compat attribute added for qcow2 for the versioning that way things are explicitly stated rather than implicitly switching versions on us. Problem is that we are linking to a library here on compile time and not executing a binary to create the image. The Ceph project is still very new, almost nobody is actually using the format 1 images anymore. I wouldn't recommend anybody creating format 1 images now. They still work just fine, but we don't want to encourage people to keep using them. Wido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
On 12/11/2013 03:47 PM, Wido den Hollander wrote: This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. Older versions of libvirt can work with this new RBD format as long as librbd supports format 2, something that all recent versions of librbd do. Did somebody get a look at this patch yet? It's a fairly simple one, but would be great for the Ceph project! Wido Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 4b6f18c..f3dd7a0 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -435,6 +435,26 @@ cleanup: return ret; } +static int virStorageBackendRBDCreateImage(rados_ioctx_t io, + char *name, long capacity) +{ +int order = 0; +#if LIBRBD_VERSION_CODE > 260 +uint64_t features = 3; +uint64_t stripe_count = 1; +uint64_t stripe_unit = 4194304; + +if (rbd_create3(io, name, capacity, features, &order, +stripe_count, stripe_unit) < 0) { +#else +if (rbd_create(io, name, capacity, &order) < 0) { +#endif +return -1; +} + +return 0; +} + static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) @@ -442,7 +462,6 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; -int order = 0; int ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", @@ -467,7 +486,7 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, goto cleanup; } -if (rbd_create(ptr.ioctx, vol->name, vol->capacity, &order) < 0) { +if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to create volume '%s/%s'"), pool->def->source.name, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. Older versions of libvirt can work with this new RBD format as long as librbd supports format 2, something that all recent versions of librbd do. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 4b6f18c..f3dd7a0 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -435,6 +435,26 @@ cleanup: return ret; } +static int virStorageBackendRBDCreateImage(rados_ioctx_t io, + char *name, long capacity) +{ +int order = 0; +#if LIBRBD_VERSION_CODE > 260 +uint64_t features = 3; +uint64_t stripe_count = 1; +uint64_t stripe_unit = 4194304; + +if (rbd_create3(io, name, capacity, features, &order, +stripe_count, stripe_unit) < 0) { +#else +if (rbd_create(io, name, capacity, &order) < 0) { +#endif +return -1; +} + +return 0; +} + static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) @@ -442,7 +462,6 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStorageBackendRBDState ptr; ptr.cluster = NULL; ptr.ioctx = NULL; -int order = 0; int ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", @@ -467,7 +486,7 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, goto cleanup; } -if (rbd_create(ptr.ioctx, vol->name, vol->capacity, &order) < 0) { +if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to create volume '%s/%s'"), pool->def->source.name, -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java] Set source and target version to Java 1.6
On 09/17/2013 02:58 PM, Claudio Bley wrote: At Tue, 17 Sep 2013 13:48:11 +0200, Wido den Hollander wrote: On 09/17/2013 01:14 PM, Claudio Bley wrote: At Tue, 17 Sep 2013 10:04:59 +0200, Wido den Hollander wrote: On platforms with Java 1.7 it will still produce compatible code with Java 1.6 platforms Java 1.6 is still out there and widely used. AFAIK, the code is 1.5 compliant which would be "least common denominator". So, there's no need raising the bar to 1.6 actually. You are correct, I tried to set target and source to 1.5 and that compiled flawlessly. The question is, do we (need to) care about 1.5? As far as I know there are no support distributions which still ship with 1.5 The platforms which ship with 1.6: - RHEL 5 and 6 - Ubuntu 10.04 and 12.04 RHEL 6 and Ubuntu 12.04 also support 1.7. But since the code is 1.5 compliant I won't complain if we set target and source to 1.5. Although I don't think there are any 1.5 users left who use libvirt-java Since there're a whole slew of JVM implementations out there I feel a bit weary bumping it up to 1.6. On the other hand, it all depends on JNA, which JVMs they support with their JNI wrapper code. Looking at the ChangeLog of JNA (https://github.com/twall/jna/blob/master/CHANGES.md#features-1) I see that they have decided to switch to 1.6 (from 1.4) in version 4.0. Considering this, I'm all for pulling the plug and go for 1.6 in libvirt, too. Cool. Let's keep it with 1.6 then. So, ACK as is. I'll push your patch shortly (since I guess you don't have commit access, do you?)... I don't have commit access indeed, so if you could push it? We (the CloudStack project) are no only facing the issue that the current 0.5.0 release is Java 1.7 which will break existing setups of users out there. Wido Claudio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java] Set source and target version to Java 1.6
On 09/17/2013 01:14 PM, Claudio Bley wrote: At Tue, 17 Sep 2013 10:04:59 +0200, Wido den Hollander wrote: On platforms with Java 1.7 it will still produce compatible code with Java 1.6 platforms Java 1.6 is still out there and widely used. AFAIK, the code is 1.5 compliant which would be "least common denominator". So, there's no need raising the bar to 1.6 actually. You are correct, I tried to set target and source to 1.5 and that compiled flawlessly. The question is, do we (need to) care about 1.5? As far as I know there are no support distributions which still ship with 1.5 The platforms which ship with 1.6: - RHEL 5 and 6 - Ubuntu 10.04 and 12.04 RHEL 6 and Ubuntu 12.04 also support 1.7. But since the code is 1.5 compliant I won't complain if we set target and source to 1.5. Although I don't think there are any 1.5 users left who use libvirt-java Wido / Claudio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 09/17/2013 11:13 AM, Daniel Veillard wrote: On Tue, Sep 17, 2013 at 10:09:36AM +0200, Wido den Hollander wrote: On 09/17/2013 09:23 AM, Daniel Veillard wrote: On Tue, Sep 17, 2013 at 09:04:46AM +0200, Wido den Hollander wrote: On 09/16/2013 07:58 AM, Daniel Veillard wrote: Raising this due to maven On Fri, Sep 13, 2013 at 05:26:24PM +0800, Daniel Veillard wrote: On Fri, Sep 13, 2013 at 11:13:53AM +0200, Wido den Hollander wrote: [...] Indeed that was the problem, I just commited the fix, thanks ! Somewhat related to this: It seems you have compiled with Java 7 causing the bindings not to work with Java 6. A lot of users are still running Java 6 on their systems. You could do this with in build.xml: Or something like: .. Java 6 is still around and I think it should be supported for older platforms. I just compiled with the java available in my system (fedora 19). Do one really need to release for every version of java potentially using libvirt-java ? Can't they just rebuild locally too ? Sure, but if the code in libvirt.org/maven2 is Java 1.7 systems building with Maven and using Java 1.6 can't use it. There is no special code in libvirt-java which requires Java 1.7 to operate. By setting the source and target version to 1.6 the code will run on at least 1.6, but will work just fine on 1.7 systems. It's mainly a compatibility thing. Okay, let's just assume i know absolutely nothing about Java use in practice :-) No problem! But the current situation is that the 0.5.0 version out there only works with Java 7. Maybe release a 0.5.1 which is compiled for Java 1.6? I take patches :-) as build.xml is in git Just sent a patch :) ACK'ed i think you can push, right ? I don't have push permissions on the libvirt-java repo. Wido Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 09/17/2013 09:23 AM, Daniel Veillard wrote: On Tue, Sep 17, 2013 at 09:04:46AM +0200, Wido den Hollander wrote: On 09/16/2013 07:58 AM, Daniel Veillard wrote: Raising this due to maven On Fri, Sep 13, 2013 at 05:26:24PM +0800, Daniel Veillard wrote: On Fri, Sep 13, 2013 at 11:13:53AM +0200, Wido den Hollander wrote: [...] Indeed that was the problem, I just commited the fix, thanks ! Somewhat related to this: It seems you have compiled with Java 7 causing the bindings not to work with Java 6. A lot of users are still running Java 6 on their systems. You could do this with in build.xml: Or something like: .. Java 6 is still around and I think it should be supported for older platforms. I just compiled with the java available in my system (fedora 19). Do one really need to release for every version of java potentially using libvirt-java ? Can't they just rebuild locally too ? Sure, but if the code in libvirt.org/maven2 is Java 1.7 systems building with Maven and using Java 1.6 can't use it. There is no special code in libvirt-java which requires Java 1.7 to operate. By setting the source and target version to 1.6 the code will run on at least 1.6, but will work just fine on 1.7 systems. It's mainly a compatibility thing. I take patches :-) as build.xml is in git Just sent a patch :) Wido Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-java] Set source and target version to Java 1.6
On platforms with Java 1.7 it will still produce compatible code with Java 1.6 platforms Java 1.6 is still out there and widely used. Signed-off-by: Wido den Hollander --- build.properties |2 ++ build.xml|4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/build.properties b/build.properties index 679a271..7721f90 100644 --- a/build.properties +++ b/build.properties @@ -2,5 +2,7 @@ version=0.5.0 release=1 libvirt.required=0.9.12 java.required=1:1.5.0 +java.target=1.6 +java.source=1.6 rpm.topdir=/home/veillard/rpms jar.dir=/usr/share/java diff --git a/build.xml b/build.xml index 61b15bb..acd9c92 100644 --- a/build.xml +++ b/build.xml @@ -52,7 +52,7 @@ destdir="target/testclasses" cache="target/cache" closure="true" /> - + @@ -60,7 +60,7 @@ destdir="target/classes" cache="target/cache" closure="true" /> - + -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 09/16/2013 07:58 AM, Daniel Veillard wrote: Raising this due to maven On Fri, Sep 13, 2013 at 05:26:24PM +0800, Daniel Veillard wrote: On Fri, Sep 13, 2013 at 11:13:53AM +0200, Wido den Hollander wrote: [...] Indeed that was the problem, I just commited the fix, thanks ! Somewhat related to this: It seems you have compiled with Java 7 causing the bindings not to work with Java 6. A lot of users are still running Java 6 on their systems. You could do this with in build.xml: Or something like: .. Java 6 is still around and I think it should be supported for older platforms. Wido Hum, getting back on-list, I pushed everything including maven files at their location, unfortunately this never shows up in repos Seems that to be indexed one need to push to an expernal repo within that small list: "Instead of maintaining repository rsync feeds for each projects, we now encourage projects to use an approved repository hosting location. Currently approved repository hosting locations: Apache Software Foundation (for all Apache projects) Codehaus (for Codehaus projects) FuseSource Forge (focused on FUSE related projects) Nuiton.org " We are definitely not Apache Software Foundation, nor Codehaus, nor FuseSource, and I have no idea what Nuiton.org might be nor intent to push to a random place. http://maven.apache.org/guides/mini/guide-central-repository-upload.html " Explanations Having each project maintain its own repository with rsync to Central Repository used to be the preferred process until January 2010. But we are no longer accepting rsync requests on a per project basis. Over time, we have learned that this process is not scalable. Many of the projects being synced release very infrequently, yet we have to hit hundreds of servers several times a day looking for artifacts that don't change. Additionally, there is no good mechanism currently for validating the incoming data via the rsync, and this leads to bad metadata that affects everyone. The aggregation of projects into single larger feeds allows us to sync faster and more often, and ensuring these locations perform sufficient checks increases the quality of metadata for everyone." As a result current repos only list libvirt-java up to 0.4.7 and none of the newer versions. Repository are evil, and they built a solution based on those. The bug is on their side, I won't upload to $random_site, so unless someone else pulls from libvirt.org, libvirt-java maven won't index more recent build. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 09/16/2013 07:58 AM, Daniel Veillard wrote: Raising this due to maven On Fri, Sep 13, 2013 at 05:26:24PM +0800, Daniel Veillard wrote: On Fri, Sep 13, 2013 at 11:13:53AM +0200, Wido den Hollander wrote: [...] Indeed that was the problem, I just commited the fix, thanks ! Hum, getting back on-list, I pushed everything including maven files at their location, unfortunately this never shows up in repos Seems that to be indexed one need to push to an expernal repo within that small list: I had this discussion last year as well with somebody else from RedHat and we ended up the the conclusion that libvirt-java wouldn't go to Maven Central. In the CloudStack project we simply added libvirt.org as a extra repository: https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=plugins/hypervisors/kvm/pom.xml;h=4c0ec982bdf1ebc6c2fc3d649759f6cfe5be88d1;hb=master#l22 I also just tried libvirt-java 0.5.0, it works for what we are using :) Wido "Instead of maintaining repository rsync feeds for each projects, we now encourage projects to use an approved repository hosting location. Currently approved repository hosting locations: Apache Software Foundation (for all Apache projects) Codehaus (for Codehaus projects) FuseSource Forge (focused on FUSE related projects) Nuiton.org " We are definitely not Apache Software Foundation, nor Codehaus, nor FuseSource, and I have no idea what Nuiton.org might be nor intent to push to a random place. http://maven.apache.org/guides/mini/guide-central-repository-upload.html " Explanations Having each project maintain its own repository with rsync to Central Repository used to be the preferred process until January 2010. But we are no longer accepting rsync requests on a per project basis. Over time, we have learned that this process is not scalable. Many of the projects being synced release very infrequently, yet we have to hit hundreds of servers several times a day looking for artifacts that don't change. Additionally, there is no good mechanism currently for validating the incoming data via the rsync, and this leads to bad metadata that affects everyone. The aggregation of projects into single larger feeds allows us to sync faster and more often, and ensuring these locations perform sufficient checks increases the quality of metadata for everyone." As a result current repos only list libvirt-java up to 0.4.7 and none of the newer versions. Repository are evil, and they built a solution based on those. The bug is on their side, I won't upload to $random_site, so unless someone else pulls from libvirt.org, libvirt-java maven won't index more recent build. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 09/13/2013 11:26 AM, Daniel Veillard wrote: On Fri, Sep 13, 2013 at 11:13:53AM +0200, Wido den Hollander wrote: On 09/13/2013 10:29 AM, Daniel Veillard wrote: [...] I'm puzzled, how can the build be successful if the main jar is not generated ??? Odd, I tried building the Deb and that works fine: wido@wido-laptop:~/repos/libvirt-java$ ant deb Buildfile: /home/wido/repos/libvirt-java/build.xml init: [mkdir] Created dir: /home/wido/repos/libvirt-java/target/classes [mkdir] Created dir: /home/wido/repos/libvirt-java/target/testclasses [mkdir] Created dir: /home/wido/repos/libvirt-java/target/cache [copy] Copying 1 file to /home/wido/repos/libvirt-java build: [javac] Compiling 64 source files to /home/wido/repos/libvirt-java/target/classes jar: [jar] Building jar: /home/wido/repos/libvirt-java/target/libvirt-0.5.0.jar deb: [mkdir] Created dir: /home/wido/repos/libvirt-java/target/libvirt-java/DEBIAN [copy] Copying 1 file to /home/wido/repos/libvirt-java/target/libvirt-java/DEBIAN [mkdir] Created dir: /home/wido/repos/libvirt-java/target/libvirt-java/usr/share/java [copy] Copying 1 file to /home/wido/repos/libvirt-java/target/libvirt-java/usr/share/java [exec] dpkg-deb: building package `libvirt-java' in `target/libvirt-java_0.5.0_all.deb'. BUILD SUCCESSFUL Total time: 3 seconds wido@wido-laptop:~/repos/libvirt-java$ The problem with the RPM seems to be it executes "ant build docs" While "build" actually builds the classes, it doesn't generate a JAR file. It should run: $ ant jar docs That produces the correct .jar file in the target directory. Indeed that was the problem, I just commited the fix, thanks ! Great! I see you tagged the 0.5.0 release. Could you also upload the files here: http://www.libvirt.org/maven2/org/libvirt/libvirt/ That way we can use Maven in CloudStack to depend on libvirt-java 0.5.0 Wido Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 09/13/2013 10:29 AM, Daniel Veillard wrote: On Mon, Sep 02, 2013 at 08:14:33AM +0800, Daniel Veillard wrote: On Fri, Aug 30, 2013 at 12:31:55PM +0200, Wido den Hollander wrote: On 08/15/2013 11:00 AM, Wido den Hollander wrote: Hello, In the recent months various new methods were added to libvirt-java which we (Apache CloudStack) would like to use in our KVM code. For example resizing storage volumes, right now we have to do this with Bash scripting since although libvirt supports resizing volumes, the current release (0.4.9) of libvirt-java doesn't. I don't know if there are any objections, but if possible I'd like to see 0.5.0 released so we get this new functionality for CloudStack. We use maven for building CloudStack and it fetches libvirt-java from libvirt.org/maven2 Can I give this one a small bump? Oops, okay, point taken, not sure i can do this today, but I will try this week ! Hi Wido, I tried to do this today, but I hit a problem, when I run ./autobuild.sh on fedora-19 it starts to build goes fine, was failing in rpm due to broken (fixed in git now), but for some reason it does not produce target/libvirt-0.5.0.jar (after fixing the build version to be 0.5.0) it does build target/libvirt-java-0.5.0.tar.gz target/libvirt-0.5.0-javadoc.jar and target/libvirt-0.5.0-sources.jar but not the binary jar. But it does seems to compile correctly: - [javac] Compiling 64 source files to /home/veillard/rpms/BUILD/libvirt-java-0.5.0/target/classes init: [copy] Copying 1 file to /home/veillard/rpms/BUILD/libvirt-java-0.5.0 build: docs: [mkdir] Created dir: /home/veillard/rpms/BUILD/libvirt-java-0.5.0/target/javadoc [javadoc] Generating Javadoc ... BUILD SUCCESSFUL Total time: 5 seconds + exit 0 --- I'm puzzled, how can the build be successful if the main jar is not generated ??? Odd, I tried building the Deb and that works fine: wido@wido-laptop:~/repos/libvirt-java$ ant deb Buildfile: /home/wido/repos/libvirt-java/build.xml init: [mkdir] Created dir: /home/wido/repos/libvirt-java/target/classes [mkdir] Created dir: /home/wido/repos/libvirt-java/target/testclasses [mkdir] Created dir: /home/wido/repos/libvirt-java/target/cache [copy] Copying 1 file to /home/wido/repos/libvirt-java build: [javac] Compiling 64 source files to /home/wido/repos/libvirt-java/target/classes jar: [jar] Building jar: /home/wido/repos/libvirt-java/target/libvirt-0.5.0.jar deb: [mkdir] Created dir: /home/wido/repos/libvirt-java/target/libvirt-java/DEBIAN [copy] Copying 1 file to /home/wido/repos/libvirt-java/target/libvirt-java/DEBIAN [mkdir] Created dir: /home/wido/repos/libvirt-java/target/libvirt-java/usr/share/java [copy] Copying 1 file to /home/wido/repos/libvirt-java/target/libvirt-java/usr/share/java [exec] dpkg-deb: building package `libvirt-java' in `target/libvirt-java_0.5.0_all.deb'. BUILD SUCCESSFUL Total time: 3 seconds wido@wido-laptop:~/repos/libvirt-java$ The problem with the RPM seems to be it executes "ant build docs" While "build" actually builds the classes, it doesn't generate a JAR file. It should run: $ ant jar docs That produces the correct .jar file in the target directory. Wido Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
On 09/12/2013 04:12 PM, Daniel P. Berrange wrote: On Thu, Sep 12, 2013 at 11:27:10AM +0200, Wido den Hollander wrote: This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. What's the compatibility like here. If one node creates a v2 image, can a host which only knows v1 still use that v2 image for booting guests ? Only very old clients won't be able to open v2 images when they only support v1. Ceph is however a fast moving project where almost all users are on the most recent versions of Ceph due to new features and bug fixes. Wido Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Use rbd_create3 to create RBD format 2 images by default
This new RBD format supports snapshotting and cloning. By having libvirt create images in format 2 end-users of the created images can benefit of the new RBD format. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index d9e1789..e79873f 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -435,6 +435,26 @@ cleanup: return ret; } +static int virStorageBackendRBDCreateImage(rados_ioctx_t io, + char *name, long capacity) +{ +int order = 0; +#if LIBRBD_VERSION_CODE > 260 +uint64_t features = 3; +uint64_t stripe_count = 1; +uint64_t stripe_unit = 4194304; + +if (rbd_create3(io, name, capacity, features, &order, +stripe_count, stripe_unit) < 0) { +#else +if (rbd_create(io, name, capacity, &order) < 0) { +#endif +return -1; +} + +return 0; +} + static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) @@ -442,7 +462,6 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, virStorageBackendRBDStatePtr ptr; ptr.cluster = NULL; ptr.ioctx = NULL; -int order = 0; int ret = -1; VIR_DEBUG("Creating RBD image %s/%s with size %llu", @@ -467,7 +486,7 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, goto cleanup; } -if (rbd_create(ptr.ioctx, vol->name, vol->capacity, &order) < 0) { +if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to create volume '%s/%s'"), pool->def->source.name, -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rbd: Use different formatter to display disk format
On 08/30/2013 12:30 PM, Wido den Hollander wrote: RBD images are always in RAW format and should be displayed that way Did anyone get the chance yet to review it? Thanks! Wido Signed-off-by: Wido den Hollander --- src/conf/storage_conf.c |3 ++- src/storage/storage_backend_rbd.c |2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 002663f..e54fc64 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -230,7 +230,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, .volOptions = { .defaultFormat = VIR_STORAGE_FILE_RAW, - .formatToString = virStoragePoolFormatDiskTypeToString, + .formatFromString = virStorageVolumeFormatFromString, + .formatToString = virStorageFileFormatTypeToString, } }, {.poolType = VIR_STORAGE_POOL_SHEEPDOG, diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index e5d720e..c435636 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -32,6 +32,7 @@ #include "base64.h" #include "viruuid.h" #include "virstring.h" +#include "virstoragefile.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -271,6 +272,7 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, vol->capacity = info.size; vol->allocation = info.obj_size * info.num_objs; vol->type = VIR_STORAGE_VOL_NETWORK; +vol->target.format = VIR_STORAGE_FILE_RAW; VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Can I request a new release of libvirt-java?
On 08/15/2013 11:00 AM, Wido den Hollander wrote: Hello, In the recent months various new methods were added to libvirt-java which we (Apache CloudStack) would like to use in our KVM code. For example resizing storage volumes, right now we have to do this with Bash scripting since although libvirt supports resizing volumes, the current release (0.4.9) of libvirt-java doesn't. I don't know if there are any objections, but if possible I'd like to see 0.5.0 released so we get this new functionality for CloudStack. We use maven for building CloudStack and it fetches libvirt-java from libvirt.org/maven2 Can I give this one a small bump? Wido Thank you, Wido den Hollander -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Use different formatter to display disk format
RBD images are always in RAW format and should be displayed that way Signed-off-by: Wido den Hollander --- src/conf/storage_conf.c |3 ++- src/storage/storage_backend_rbd.c |2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 002663f..e54fc64 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -230,7 +230,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, .volOptions = { .defaultFormat = VIR_STORAGE_FILE_RAW, - .formatToString = virStoragePoolFormatDiskTypeToString, + .formatFromString = virStorageVolumeFormatFromString, + .formatToString = virStorageFileFormatTypeToString, } }, {.poolType = VIR_STORAGE_POOL_SHEEPDOG, diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index e5d720e..c435636 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -32,6 +32,7 @@ #include "base64.h" #include "viruuid.h" #include "virstring.h" +#include "virstoragefile.h" #include "rados/librados.h" #include "rbd/librbd.h" @@ -271,6 +272,7 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, vol->capacity = info.size; vol->allocation = info.obj_size * info.num_objs; vol->type = VIR_STORAGE_VOL_NETWORK; +vol->target.format = VIR_STORAGE_FILE_RAW; VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Can I request a new release of libvirt-java?
Hello, In the recent months various new methods were added to libvirt-java which we (Apache CloudStack) would like to use in our KVM code. For example resizing storage volumes, right now we have to do this with Bash scripting since although libvirt supports resizing volumes, the current release (0.4.9) of libvirt-java doesn't. I don't know if there are any objections, but if possible I'd like to see 0.5.0 released so we get this new functionality for CloudStack. We use maven for building CloudStack and it fetches libvirt-java from libvirt.org/maven2 Thank you, Wido den Hollander -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rbd: Do not free the secret if it is not set
Not all RBD (Ceph) storage pools have cephx authentication turned on, so "secret" might not be initialized. It could also be that the secret couldn't be located. Only call virSecretFree() if "secret" is initialized earlier. Signed-off-by: Wido den Hollander --- src/storage/storage_backend_rbd.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 493e33b..1f75481 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -176,7 +176,11 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr, cleanup: VIR_FREE(secret_value); VIR_FREE(rados_key); -virSecretFree(secret); + +if (secret != NULL) { +virSecretFree(secret); +} + virBufferFreeAndReset(&mon_host); VIR_FREE(mon_buff); return ret; -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't require a block or file when looking for an alias.
Did this one get lost or forgotten? Thanks, Wido On 04/05/2013 06:07 PM, Wido den Hollander wrote: This for example prohibits you to use iotune for Ceph or Sheepdog devices. Signed-off-by: Wido den Hollander --- src/qemu/qemu_driver.c |4 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 552a81b..464d30a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13032,10 +13032,6 @@ qemuDiskPathToAlias(virDomainObjPtr vm, const char *path, int *idx) if (idx) *idx = i; -if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && -disk->type != VIR_DOMAIN_DISK_TYPE_FILE) -goto cleanup; - if (disk->src) { if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0) { virReportOOMError(); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't require a block or file when looking for an alias.
This for example prohibits you to use iotune for Ceph or Sheepdog devices. Signed-off-by: Wido den Hollander --- src/qemu/qemu_driver.c |4 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 552a81b..464d30a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13032,10 +13032,6 @@ qemuDiskPathToAlias(virDomainObjPtr vm, const char *path, int *idx) if (idx) *idx = i; -if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && -disk->type != VIR_DOMAIN_DISK_TYPE_FILE) -goto cleanup; - if (disk->src) { if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0) { virReportOOMError(); -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list