[libvirt] [java PATCH 1/1] Add support for Qemu Guest Agent commands

2018-07-19 Thread Wido den Hollander
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 ***

2018-07-19 Thread Wido den Hollander
*** 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

2017-06-16 Thread Wido den Hollander
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?

2016-07-02 Thread Wido den Hollander

> 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

2016-02-11 Thread Wido den Hollander
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

2016-02-11 Thread Wido den Hollander
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

2016-02-11 Thread Wido den Hollander
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

2016-02-11 Thread Wido den Hollander
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

2016-02-10 Thread Wido den Hollander
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

2016-02-01 Thread Wido den Hollander

> 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

2016-01-31 Thread Wido den Hollander


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

2016-01-30 Thread Wido den Hollander
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

2016-01-30 Thread Wido den Hollander
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

2016-01-30 Thread Wido den Hollander
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

2016-01-30 Thread Wido den Hollander
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

2016-01-28 Thread Wido den Hollander
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.

2016-01-27 Thread Wido den Hollander
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

2016-01-27 Thread Wido den Hollander
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

2016-01-27 Thread Wido den Hollander
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

2016-01-27 Thread Wido den Hollander
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

2016-01-27 Thread Wido den Hollander
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

2016-01-27 Thread Wido den Hollander
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

2016-01-22 Thread Wido den Hollander


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

2016-01-15 Thread Wido den Hollander
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()

2016-01-13 Thread Wido den Hollander
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

2016-01-13 Thread Wido den Hollander
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

2016-01-06 Thread Wido den Hollander


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

2016-01-06 Thread Wido den Hollander
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

2016-01-06 Thread Wido den Hollander


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

2016-01-06 Thread Wido den Hollander
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

2016-01-06 Thread Wido den Hollander
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

2016-01-06 Thread Wido den Hollander
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

2016-01-05 Thread Wido den Hollander


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

2015-12-29 Thread Wido den Hollander
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

2015-12-28 Thread Wido den Hollander
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

2015-12-25 Thread Wido den Hollander
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

2015-12-25 Thread Wido den Hollander
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

2015-12-24 Thread Wido den Hollander
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()

2015-12-23 Thread Wido den Hollander
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()

2015-12-23 Thread Wido den Hollander


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

2015-12-23 Thread Wido den Hollander


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

2015-12-23 Thread Wido den Hollander


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

2015-12-23 Thread Wido den Hollander
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()

2015-12-23 Thread Wido den Hollander
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

2015-12-23 Thread Wido den Hollander
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

2015-12-18 Thread Wido den Hollander


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

2015-11-24 Thread Wido den Hollander


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

2015-11-16 Thread Wido den Hollander


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

2015-11-13 Thread Wido den Hollander


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

2015-11-09 Thread Wido den Hollander
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

2015-11-06 Thread Wido den Hollander
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

2015-10-27 Thread Wido den Hollander
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

2015-10-23 Thread Wido den Hollander
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

2015-10-15 Thread Wido den Hollander
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

2015-10-15 Thread Wido den Hollander


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

2015-10-15 Thread Wido den Hollander
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

2015-09-14 Thread Wido den Hollander
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

2015-09-07 Thread Wido den Hollander


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

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

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

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

Wido

> --
> Claudio
> 

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


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

2015-09-02 Thread Wido den Hollander


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

2015-09-02 Thread Wido den Hollander


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)

2015-08-28 Thread Wido den Hollander

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

2015-08-28 Thread Wido den Hollander
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.

2015-07-16 Thread Wido den Hollander


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.

2015-07-14 Thread Wido den Hollander
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

2014-06-13 Thread Wido den Hollander



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

2014-05-28 Thread Wido den Hollander
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

2014-05-28 Thread Wido den Hollander
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

2014-05-28 Thread Wido den Hollander
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

2014-05-28 Thread Wido den Hollander
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

2014-02-25 Thread Wido den Hollander
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

2014-02-25 Thread Wido den Hollander
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

2014-02-25 Thread Wido den Hollander
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

2014-02-12 Thread Wido den Hollander
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

2014-02-12 Thread Wido den Hollander
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

2014-02-12 Thread Wido den Hollander
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

2014-02-12 Thread Wido den Hollander
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

2014-02-04 Thread Wido den Hollander



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

2014-01-30 Thread Wido den Hollander
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

2014-01-13 Thread Wido den Hollander

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

2014-01-04 Thread Wido den Hollander



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

2014-01-02 Thread Wido den Hollander



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

2013-12-11 Thread Wido den Hollander
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

2013-09-17 Thread Wido den Hollander

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

2013-09-17 Thread Wido den Hollander

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?

2013-09-17 Thread Wido den Hollander

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?

2013-09-17 Thread Wido den Hollander

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

2013-09-17 Thread Wido den Hollander
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?

2013-09-17 Thread Wido den Hollander

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?

2013-09-16 Thread Wido den Hollander



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?

2013-09-13 Thread Wido den Hollander



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?

2013-09-13 Thread Wido den Hollander



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

2013-09-12 Thread Wido den Hollander



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

2013-09-12 Thread Wido den Hollander
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

2013-09-12 Thread Wido den Hollander

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?

2013-08-30 Thread Wido den Hollander

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

2013-08-30 Thread Wido den Hollander
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?

2013-08-15 Thread Wido den Hollander

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

2013-07-16 Thread Wido den Hollander
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.

2013-04-24 Thread Wido den Hollander

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.

2013-04-05 Thread Wido den Hollander
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


  1   2   >