7:56 a.m.
From: Alexander Polyakov <apolyakov(a)beget.com>
Includes suggestions by John Ferlan & Peter Krempa
Alexander Polyakov (2):
logical: implement volume resize
news: Document lvm volume resize
docs/news.xml | 8 ++++++++
m4/virt-storage-lvm.m4 | 4 ++++
src/storage/storage_backend_logical.c | 37 +++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
--
2.13.5
7:56 a.m.
New subject: [libvirt] [PATCH 1/2] logical: implement volume resize
From: Alexander Polyakov <apolyakov(a)beget.com>
Add new virStorageBackendLogicalResizeVol function to
implement resize of logical volumes
Sparse volumes are not supported
Signed-off-by: Alexander Polyakov <apolyakov(a)beget.com>
---
m4/virt-storage-lvm.m4 | 4 ++++
src/storage/storage_backend_logical.c | 37 +++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/m4/virt-storage-lvm.m4 b/m4/virt-storage-lvm.m4
index a0ccca7a0..0932995b4 100644
--- a/m4/virt-storage-lvm.m4
+++ b/m4/virt-storage-lvm.m4
@@ -30,6 +30,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
AC_PATH_PROG([VGREMOVE], [vgremove], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([LVREMOVE], [lvremove], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([LVCHANGE], [lvchange], [], [$LIBVIRT_SBIN_PATH])
+ AC_PATH_PROG([LVRESIZE], [lvresize], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([VGCHANGE], [vgchange], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([VGSCAN], [vgscan], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([PVS], [pvs], [], [$LIBVIRT_SBIN_PATH])
@@ -44,6 +45,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
if test -z "$VGREMOVE" ; then AC_MSG_ERROR([We need vgremove for LVM
storage driver]) ; fi
if test -z "$LVREMOVE" ; then AC_MSG_ERROR([We need lvremove for LVM
storage driver]) ; fi
if test -z "$LVCHANGE" ; then AC_MSG_ERROR([We need lvchange for LVM
storage driver]) ; fi
+ if test -z "$LVRESIZE" ; then AC_MSG_ERROR([We need lvresize for LVM
storage driver]) ; fi
if test -z "$VGCHANGE" ; then AC_MSG_ERROR([We need vgchange for LVM
storage driver]) ; fi
if test -z "$VGSCAN" ; then AC_MSG_ERROR([We need vgscan for LVM storage
driver]) ; fi
if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage
driver]) ; fi
@@ -57,6 +59,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
if test -z "$VGREMOVE" ; then with_storage_lvm=no ; fi
if test -z "$LVREMOVE" ; then with_storage_lvm=no ; fi
if test -z "$LVCHANGE" ; then with_storage_lvm=no ; fi
+ if test -z "$LVRESIZE" ; then with_storage_lvm=no ; fi
if test -z "$VGCHANGE" ; then with_storage_lvm=no ; fi
if test -z "$VGSCAN" ; then with_storage_lvm=no ; fi
if test -z "$PVS" ; then with_storage_lvm=no ; fi
@@ -75,6 +78,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
AC_DEFINE_UNQUOTED([VGREMOVE],["$VGREMOVE"],[Location of vgremove
program])
AC_DEFINE_UNQUOTED([LVREMOVE],["$LVREMOVE"],[Location of lvremove
program])
AC_DEFINE_UNQUOTED([LVCHANGE],["$LVCHANGE"],[Location of lvchange
program])
+ AC_DEFINE_UNQUOTED([LVRESIZE],["$LVRESIZE"],[Location of lvresize
program])
AC_DEFINE_UNQUOTED([VGCHANGE],["$VGCHANGE"],[Location of vgchange
program])
AC_DEFINE_UNQUOTED([VGSCAN],["$VGSCAN"],[Location of vgscan program])
AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program])
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 67f70e551..e5cb09922 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -1072,6 +1072,42 @@ virStorageBackendLogicalVolWipe(virConnectPtr conn,
return -1;
}
+static int
+virStorageBackendLogicalResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ virStorageVolDefPtr vol,
+ unsigned long long capacity,
+ unsigned int flags)
+{
+ virCheckFlags(VIR_STORAGE_VOL_RESIZE_SHRINK |
+ VIR_STORAGE_VOL_RESIZE_DELTA, -1);
+
+ bool delta = flags & VIR_STORAGE_VOL_RESIZE_DELTA;
+ bool shrink = flags & VIR_STORAGE_VOL_RESIZE_SHRINK;
+
+ if (vol->target.sparse) {
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("logical volume '%s' is sparse, volume resize
"
+ "not supported"),
+ vol->target.path);
+ return -1;
+ }
+
+ virCommandPtr cmd = virCommandNewArgList(LVRESIZE, "-L", NULL);
+
+ if (delta) {
+ virCommandAddArgFormat(cmd, "%c%lluB", shrink ? '-' :
'+',
+ capacity);
+ } else {
+ virCommandAddArgFormat(cmd, "%lluB", capacity);
+ }
+ virCommandAddArgFormat(cmd, "%s", vol->target.path);
+ int ret = virCommandRun(cmd, NULL);
+
+ virCommandFree(cmd);
+ return ret;
+}
+
virStorageBackend virStorageBackendLogical = {
.type = VIR_STORAGE_POOL_LOGICAL,
@@ -1089,6 +1125,7 @@ virStorageBackend virStorageBackendLogical = {
.uploadVol = virStorageBackendVolUploadLocal,
.downloadVol = virStorageBackendVolDownloadLocal,
.wipeVol = virStorageBackendLogicalVolWipe,
+ .resizeVol = virStorageBackendLogicalResizeVol,
};
--
2.13.5
8:29 p.m.
New subject: [libvirt] [PATCH 1/2] logical: implement volume resize
On 09/07/2017 08:56 AM, apolyakov(a)beget.ru wrote:
From: Alexander Polyakov <apolyakov(a)beget.com>
Add new virStorageBackendLogicalResizeVol function to
implement resize of logical volumes
Sparse volumes are not supported
Signed-off-by: Alexander Polyakov <apolyakov(a)beget.com>
---
m4/virt-storage-lvm.m4 | 4 ++++
src/storage/storage_backend_logical.c | 37 +++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
Curious - is there a way to create/modify an LV to use LUKS encryption
via some lvm* commands? I have another bz and the only way that comes
to me right now is to create the LV using qemu-img with and then somehow
have it recognized/added so that LVM recognizes it. I haven't put much
thought into it yet though...
Anyway...
diff --git a/m4/virt-storage-lvm.m4 b/m4/virt-storage-lvm.m4
index a0ccca7a0..0932995b4 100644
--- a/m4/virt-storage-lvm.m4
+++ b/m4/virt-storage-lvm.m4
@@ -30,6 +30,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
AC_PATH_PROG([VGREMOVE], [vgremove], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([LVREMOVE], [lvremove], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([LVCHANGE], [lvchange], [], [$LIBVIRT_SBIN_PATH])
+ AC_PATH_PROG([LVRESIZE], [lvresize], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([VGCHANGE], [vgchange], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([VGSCAN], [vgscan], [], [$LIBVIRT_SBIN_PATH])
AC_PATH_PROG([PVS], [pvs], [], [$LIBVIRT_SBIN_PATH])
@@ -44,6 +45,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
if test -z "$VGREMOVE" ; then AC_MSG_ERROR([We need vgremove for LVM
storage driver]) ; fi
if test -z "$LVREMOVE" ; then AC_MSG_ERROR([We need lvremove for LVM
storage driver]) ; fi
if test -z "$LVCHANGE" ; then AC_MSG_ERROR([We need lvchange for LVM
storage driver]) ; fi
+ if test -z "$LVRESIZE" ; then AC_MSG_ERROR([We need lvresize for LVM
storage driver]) ; fi
if test -z "$VGCHANGE" ; then AC_MSG_ERROR([We need vgchange for LVM
storage driver]) ; fi
if test -z "$VGSCAN" ; then AC_MSG_ERROR([We need vgscan for LVM storage
driver]) ; fi
if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage
driver]) ; fi
@@ -57,6 +59,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
if test -z "$VGREMOVE" ; then with_storage_lvm=no ; fi
if test -z "$LVREMOVE" ; then with_storage_lvm=no ; fi
if test -z "$LVCHANGE" ; then with_storage_lvm=no ; fi
+ if test -z "$LVRESIZE" ; then with_storage_lvm=no ; fi
if test -z "$VGCHANGE" ; then with_storage_lvm=no ; fi
if test -z "$VGSCAN" ; then with_storage_lvm=no ; fi
if test -z "$PVS" ; then with_storage_lvm=no ; fi
@@ -75,6 +78,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_LVM], [
AC_DEFINE_UNQUOTED([VGREMOVE],["$VGREMOVE"],[Location of vgremove
program])
AC_DEFINE_UNQUOTED([LVREMOVE],["$LVREMOVE"],[Location of lvremove
program])
AC_DEFINE_UNQUOTED([LVCHANGE],["$LVCHANGE"],[Location of lvchange
program])
+ AC_DEFINE_UNQUOTED([LVRESIZE],["$LVRESIZE"],[Location of lvresize
program])
AC_DEFINE_UNQUOTED([VGCHANGE],["$VGCHANGE"],[Location of vgchange
program])
AC_DEFINE_UNQUOTED([VGSCAN],["$VGSCAN"],[Location of vgscan program])
AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program])
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 67f70e551..e5cb09922 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -1072,6 +1072,42 @@ virStorageBackendLogicalVolWipe(virConnectPtr conn,
return -1;
}
I see the lvmresize command allows you to supply which physical device
the extents would come from. Would that be something we might want to
try here as well? Thankfully I don't think snapshot or mirror'd LV's
come into play (yet).... nor thin pool or stripes.
In any case, I don't know whether someone would "want" to choose a
specific PV to draw from or just let LVM make the decision and let it be
for now. If we did have another parameter we'd have to check if the PV
existed and was part of the VG... lot's more work, but didn't want to
miss noting it either. Just typing some thoughts...
+static int
+virStorageBackendLogicalResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ virStorageVolDefPtr vol,
+ unsigned long long capacity,
+ unsigned int flags)
^
The spacing for each of the argument rows is off by a single space.
+{
+ virCheckFlags(VIR_STORAGE_VOL_RESIZE_SHRINK |
+ VIR_STORAGE_VOL_RESIZE_DELTA, -1);
+
+ bool delta = flags & VIR_STORAGE_VOL_RESIZE_DELTA;
+ bool shrink = flags & VIR_STORAGE_VOL_RESIZE_SHRINK;
Definitions go before the virCheckFlags - yes, I know the code isn't
consistent, but new code can be.
+
+ if (vol->target.sparse) {
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("logical volume '%s' is sparse, volume resize
"
+ "not supported"),
^^
Off by 2 spaces here
+ vol->target.path);
+ return -1;
+ }
+
+ virCommandPtr cmd = virCommandNewArgList(LVRESIZE, "-L", NULL);
virCommandPtr cmd; needs to be at the top...
+
+ if (delta) {
+ virCommandAddArgFormat(cmd, "%c%lluB", shrink ? '-' :
'+',
+ capacity);
Off by 2 spaces here too, but this can be on the previous line....
+ } else {
+ virCommandAddArgFormat(cmd, "%lluB", capacity);
+ }
... meaning the { } { } are not necessary
Not that any other backend specifically checks... Should there be any
concern that capacity - shrink < 0 - I assume LVM may choke on that
right? Makes me wonder if this code should add a &status to the
virCommandRun below and then a check that the returned status is valid;
otherwise, it could appear that the command succeeded when in fact it
didn't. Yes, that's a departure from other commands in this module, but
that doesn't mean they're necessarily right either.
+ virCommandAddArgFormat(cmd, "%s",
vol->target.path);
+ int ret = virCommandRun(cmd, NULL);
int ret; at the top
I can fix the nit spacing things before pushing.
Let me know your thoughts on altering the virCommandRun to change NULL
to &status and then issue some sort of error:
if (status != 0) {
virReportError(...)
...
}
NB: I didn't test with what status could return, I'm assuming == 0 is
good status, anything else is bad status.
The ReportError could be something to the effect that the command
failed. If you add and output and error buffer (e.g.
virCommandSet{Output|Error}Buffer the error from the command can be
attached.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
+
+ virCommandFree(cmd);
+ return ret;
+}
+
virStorageBackend virStorageBackendLogical = {
.type = VIR_STORAGE_POOL_LOGICAL,
@@ -1089,6 +1125,7 @@ virStorageBackend virStorageBackendLogical = {
.uploadVol = virStorageBackendVolUploadLocal,
.downloadVol = virStorageBackendVolDownloadLocal,
.wipeVol = virStorageBackendLogicalVolWipe,
+ .resizeVol = virStorageBackendLogicalResizeVol,
};
7:56 a.m.
New subject: [libvirt] [PATCH 2/2] news: Document lvm volume resize
From: Alexander Polyakov <apolyakov(a)beget.com>
Signed-off-by: Alexander Polyakov <apolyakov(a)beget.com>
---
docs/news.xml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index 483f9d6d1..44e99205f 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,14 @@
<section title="New features">
</section>
<section title="Improvements">
+ <change>
+ <summary>
+ logical: Add volume resize
+ </summary>
+ <description>
+ It's now possible to resize LVM volumes.
+ </description>
+ </change>
</section>
<section title="Bug fixes">
</section>
--
2.13.5
8:29 p.m.
New subject: [libvirt] [PATCH 2/2] news: Document lvm volume resize
On 09/07/2017 08:56 AM, apolyakov(a)beget.ru wrote:
From: Alexander Polyakov <apolyakov(a)beget.com>
Signed-off-by: Alexander Polyakov <apolyakov(a)beget.com>
---
docs/news.xml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index 483f9d6d1..44e99205f 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,14 @@
<section title="New features">
</section>
<section title="Improvements">
+ <change>
+ <summary>
+ logical: Add volume resize
+ </summary>
+ <description>
+ It's now possible to resize LVM volumes.
I would probably add "non sparse" LVM volumes... Just to be clear.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
+ </description>
+ </change>
</section>
<section title="Bug fixes">
</section>
2621
days inactive
2633
days old
4 comments
2 participants
participants (2)
-
apolyakov＠beget.ru -
John Ferlan