9:23 p.m.
On 01/27/2012 04:40 PM, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org>
Add a new function to allow changing of capacity of storage volumes.
I know you still have to rebase, but here's some more review comments:
diff --git a/src/storage/storage_backend.h
b/src/storage/storage_backend.h
index 75ed676..a37bf7c 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -44,6 +44,11 @@ typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn,
virStoragePoolObjP
typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStoragePoolObjPtr
pool,
virStorageVolDefPtr origvol,
virStorageVolDefPtr newvol,
unsigned int flags);
+typedef int (*virStorageBackendVolumeResize)(virConnectPtr conn,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ unsigned long long capacity,
You'll want to change this to long long.
+ unsigned int flags);
+static int
+virStorageBackendFilesystemResizeQemuImg(const char *path,
+ unsigned long long capacity)
+{
+ int ret = -1;
+ char *img_tool;
+ virCommandPtr cmd = NULL;
+
+ /* KVM is usually ahead of qemu on features, so try that first */
+ img_tool = virFindFileInPath("kvm-img");
+ if (!img_tool)
+ img_tool = virFindFileInPath("qemu-img");
I'm surprised we're not caching this in some helper function. Oh well,
that's an independent cleanup.
+
+ cmd = virCommandNew(img_tool);
+ virCommandAddArgList(cmd, "resize", path, NULL);
+ virCommandAddArgFormat(cmd, "%llu", capacity);
Guess what - qemu-img can do delta resizing, as well as shrinking. In
fact, qemu-img can even do (sparse-only) resizing of raw files! But for
raw files, I think we are better off doing it ourselves (as your patch
already did), as it means fewer forks and more control over whether the
result is sparse.
qemu-img resize foo 10M # reset size to 10M, regardless of whether that
is bigger or smaller - which means you have to check the capacity
yourself before calling qemu-img with a smaller capacity
qemu-img resize foo +10M # add 10M to capacity
qemu-img resize foo -10M # remove 10M from capacity
That all means that you have to be careful of the incoming flags, and
control whether you output a sign in the command line argument.
+/**
+ * Resize a volume
+ */
+static int
+virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ virStorageVolDefPtr vol,
+ unsigned long long capacity,
+ unsigned int flags)
+{
+ virCheckFlags(0, -1);
+
+ if (vol->target.format == VIR_STORAGE_FILE_RAW) {
+ return virStorageFileResize(vol->target.path, capacity);
That signature loses the notion of whether you are doing a delta or an
absolute change. Either you need to convert delta into absolute before
forwarding lower down the chain, or you need to add a bool parameter
stating whether a delta change is desired.
+ } else if (vol->target.format == VIR_STORAGE_FILE_DIR) {
+ virStorageReportError(VIR_ERR_NO_SUPPORT,
+ "%s", _("Changing size of directory based
"
+ "volumes is not supported"));
+ return -1;
@@ -1199,6 +1255,7 @@ virStorageBackend virStorageBackendDirectory =
{
.createVol = virStorageBackendFileSystemVolCreate,
.refreshVol = virStorageBackendFileSystemVolRefresh,
.deleteVol = virStorageBackendFileSystemVolDelete,
+ .resizeVol = virStorageBackendFileSystemVolResize,
I'd leave this one out. That way, you don't have to handle
VIR_STORAGE_FILE_DIR in virStorageBackendFileSystemVolResize in the
first place.
+static int
+storageVolumeResize(virStorageVolPtr obj,
+ unsigned long long capacity,
+ unsigned int flags)
+{
+ virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
+ virStorageBackendPtr backend;
+ virStoragePoolObjPtr pool = NULL;
+ virStorageVolDefPtr vol = NULL;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
Make sure you permit the flags you plan on supporting in at least one
backend (and it's okay if you don't support them all up front).
+++ b/src/util/storage_file.c
@@ -931,6 +931,22 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta)
VIR_FREE(meta);
}
+/**
+ * virStorageFileResize:
+ *
+ * Change the capacity of the raw storage file at 'path'.
+ */
+int
+virStorageFileResize(const char *path, unsigned long long capacity)
+{
+ if (truncate(path, capacity) < 0) {
+ virReportSystemError(errno, _("Failed to truncate file '%s'"),
path);
+ return -1;
+ }
Here, if the allocate flag is specified, you'd want two implementations:
If posix_fallocate exists, then open() the file, and use posix_fallocate
to ensure that the delta from the old size to the new size is allocated.
If it does not exist, then you need a while loop that writes a '\0' byte
every 4096 bytes or so, in order to force the capacity increase to be
non-sparse.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org