[PATCH 00/10] qemu: Improve disk resizing on block devices and support resizing of disk with slice

This series implements two features: - automatic size discovery when resizing a raw disk on a block device - resizing of raw disks when a 'storage slice' is in use Peter Krempa (10): qemu: block: Introduce helpers for properly testing for 'raw' and 'luks' images qemu: Use qemuBlockStorageSourceIsQEMULuks/qemuBlockStorageSourceIsRaw qemuDomainBlockResize: Agregate all checks at the beginning vsh: Introduce simple version of VSH_ALTERNATIVE_OPTIONS_EXPR virDomainBlockResize: Introduce VIR_DOMAIN_BLOCK_RESIZE_CAPACITY qemuDomainBlockResize: Implement VIR_DOMAIN_BLOCK_RESIZE_CAPACITY qemu: block: Make 'slice' layer effective for 'raw' storage source qemu: block: Format storage slice properties optionally virStorageSourceSliceFree: Export function qemuDomainBlockResize: Properly resize disks with storage slice docs/manpages/virsh.rst | 6 +- include/libvirt/libvirt-domain.h | 1 + src/conf/storage_source_conf.c | 2 +- src/conf/storage_source_conf.h | 2 + src/libvirt-domain.c | 5 + src/libvirt_private.syms | 1 + src/qemu/qemu_block.c | 193 +++++++++++++----- src/qemu/qemu_block.h | 9 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 99 ++++++++- .../disk-slices.x86_64-latest.args | 14 +- tools/virsh-domain.c | 10 +- tools/vsh.h | 4 + 13 files changed, 276 insertions(+), 72 deletions(-) -- 2.43.0

Unfortunately a LUKS image to be decrypted by qemu has VIR_STORAGE_FILE_RAW as format, but has encryption properties populated. Many places in the code don't check it properly and also don't check properly whether the image is indeed LUKS to be decrypted by qemu. Introduce helpes which will simplify this task. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 5 +++++ 2 files changed, 48 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7e9daf0bdc..845b273b27 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3237,6 +3237,49 @@ qemuBlockReopenReadOnly(virDomainObj *vm, return qemuBlockReopenAccess(vm, src, true, asyncJob); } + +/** + * qemuBlockStorageSourceIsQEMULuks: + * @src: storage source object + * + * Returns true if @src is an image in 'luks' format, which is to be decrypted + * in qemu (rather than transparently by the transport layer or host's kernel). + */ +bool +qemuBlockStorageSourceIsQEMULuks(const virStorageSource *src) +{ + if (src->format != VIR_STORAGE_FILE_RAW) + return false; + + if (src->encryption && + src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_QEMU && + src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + return true; + + return false; +} + + +/** + * qemuBlockStorageSourceIsRaw: + * @src: storage source object + * + * Returns true if @src is a true 'raw' image. This specifically excludes + * LUKS encrypted images to be decrypted by qemu. + */ +bool +qemuBlockStorageSourceIsRaw(const virStorageSource *src) +{ + if (src->format != VIR_STORAGE_FILE_RAW) + return false; + + if (qemuBlockStorageSourceIsQEMULuks(src)) + return false; + + return true; +} + + /** * qemuBlockStorageSourceNeedSliceLayer: * @src: source to inspect diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 0eab0d822c..9d6167b6ef 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -267,6 +267,11 @@ qemuBlockReopenReadOnly(virDomainObj *vm, virStorageSource *src, virDomainAsyncJob asyncJob); +bool +qemuBlockStorageSourceIsQEMULuks(const virStorageSource *src); +bool +qemuBlockStorageSourceIsRaw(const virStorageSource *src); + bool qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource *src); -- 2.43.0

On a Thursday in 2023, Peter Krempa wrote:
Unfortunately a LUKS image to be decrypted by qemu has VIR_STORAGE_FILE_RAW as format, but has encryption properties populated.
Many places in the code don't check it properly and also don't check properly whether the image is indeed LUKS to be decrypted by qemu.
Introduce helpes which will simplify this task.
*helpers
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 5 +++++ 2 files changed, 48 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7e9daf0bdc..845b273b27 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3237,6 +3237,49 @@ qemuBlockReopenReadOnly(virDomainObj *vm, return qemuBlockReopenAccess(vm, src, true, asyncJob); }
+ +/** + * qemuBlockStorageSourceIsQEMULuks:
This would be the only function with "Luks" capitalized like that in the whole repo. qemuBlockStorageSourceIsQEMULUKS looks unreadable. Would qemuBlockStorageSourceIsLUKS be enough? We're already talking about "qemu Block Storage" Either way: Reviewed-by: Ján Tomko <jtomko@redhat.com> and with the typo fixed: Spellchecked-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, Dec 14, 2023 at 13:14:43 +0100, Ján Tomko wrote:
On a Thursday in 2023, Peter Krempa wrote:
Unfortunately a LUKS image to be decrypted by qemu has VIR_STORAGE_FILE_RAW as format, but has encryption properties populated.
Many places in the code don't check it properly and also don't check properly whether the image is indeed LUKS to be decrypted by qemu.
Introduce helpes which will simplify this task.
*helpers
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 5 +++++ 2 files changed, 48 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7e9daf0bdc..845b273b27 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3237,6 +3237,49 @@ qemuBlockReopenReadOnly(virDomainObj *vm, return qemuBlockReopenAccess(vm, src, true, asyncJob); }
+ +/** + * qemuBlockStorageSourceIsQEMULuks:
This would be the only function with "Luks" capitalized like that in the whole repo.
qemuBlockStorageSourceIsQEMULUKS looks unreadable. Would qemuBlockStorageSourceIsLUKS be enough? We're already talking about "qemu Block Storage"
This function checks for LUKS images which are decrypted by qemu itself. There is another instance where the LUKS decryption is done in the storage access library (ceph). I can go with IsLuks and make sure it's very obviously documented.

Refactor code checking whether image is raw. This fixes multiple places where a LUKS encrypted disk could be mistreated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 32 ++++++++++++-------------------- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 845b273b27..83954690d6 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -177,7 +177,7 @@ bool qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSource *src) { /* no need to check in backing chain since only RAW storage supports this */ - return src->format == VIR_STORAGE_FILE_RAW; + return qemuBlockStorageSourceIsRaw(src); } @@ -1336,10 +1336,12 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSource *src) case VIR_STORAGE_FILE_FAT: /* The fat layer is emulated by the storage access layer, so we need to * put a raw layer on top */ + if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0) + return NULL; + break; + case VIR_STORAGE_FILE_RAW: - if (src->encryption && - src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_QEMU && - src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { + if (qemuBlockStorageSourceIsQEMULuks(src)) { if (qemuBlockStorageSourceGetFormatLUKSProps(src, props) < 0) return NULL; } else { @@ -2080,9 +2082,7 @@ qemuBlockStorageSourceCreateAddBacking(virStorageSource *backing, return 0; if (format) { - if (backing->format == VIR_STORAGE_FILE_RAW && - backing->encryption && - backing->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + if (qemuBlockStorageSourceIsQEMULuks(backing)) backingFormatStr = "luks"; else backingFormatStr = virStorageFileFormatTypeToString(backing->format); @@ -2313,8 +2313,7 @@ qemuBlockStorageSourceCreateGetFormatProps(virStorageSource *src, { switch ((virStorageFileFormat) src->format) { case VIR_STORAGE_FILE_RAW: - if (!src->encryption || - src->encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + if (!qemuBlockStorageSourceIsQEMULuks(src)) return 0; return qemuBlockStorageSourceCreateGetFormatPropsLUKS(src, props); @@ -2584,8 +2583,8 @@ qemuBlockStorageSourceCreateFormat(virDomainObj *vm, g_autoptr(virJSONValue) createformatprops = NULL; int ret; - if (src->format == VIR_STORAGE_FILE_RAW && - !src->encryption) + /* we don't bother creating only a true 'raw' image */ + if (qemuBlockStorageSourceIsRaw(src)) return 0; if (qemuBlockStorageSourceCreateGetFormatProps(src, backingStore, @@ -2743,7 +2742,7 @@ qemuBlockStorageSourceCreateDetectSize(GHashTable *blockNamedNodeData, } } - if (src->format == VIR_STORAGE_FILE_RAW) { + if (qemuBlockStorageSourceIsRaw(src)) { src->physical = entry->capacity; } else { src->physical = entry->physical; @@ -3299,14 +3298,7 @@ qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource *src) if (!src->sliceStorage) return false; - if (src->format != VIR_STORAGE_FILE_RAW) - return true; - - if (src->encryption && - src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) - return true; - - return false; + return !qemuBlockStorageSourceIsRaw(src); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 23909dbbab..47b18923c0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1631,7 +1631,7 @@ qemuBuildDriveSourceStr(virDomainDiskDef *disk, virBufferAddLit(buf, ","); if (encinfo) { - if (disk->src->format == VIR_STORAGE_FILE_RAW) { + if (qemuBlockStorageSourceIsQEMULuks(disk->src)) { virBufferAsprintf(buf, "key-secret=%s,", encinfo[0]->alias); rawluks = true; } else if (disk->src->format == VIR_STORAGE_FILE_QCOW2 && diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9331369d4d..4a21e5af9c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10090,7 +10090,7 @@ qemuDomainBlockPeek(virDomainPtr dom, goto cleanup; } - if (disk->src->format != VIR_STORAGE_FILE_RAW) { + if (qemuBlockStorageSourceIsRaw(disk->src)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("peeking is only supported for disk with 'raw' format not '%1$s'"), virStorageFileFormatTypeToString(disk->src->format)); @@ -10285,7 +10285,7 @@ qemuStorageLimitsRefresh(virQEMUDriverConfig *cfg, * query the highest allocated extent from QEMU */ if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_BLOCK && - src->format != VIR_STORAGE_FILE_RAW && + !qemuBlockStorageSourceIsRaw(src) && S_ISBLK(sb.st_mode)) src->allocation = 0; -- 2.43.0

Move the check for readonly and empty disks to the top where all other checks will be done. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a21e5af9c..7f719c33de 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9279,6 +9279,13 @@ qemuDomainBlockResize(virDomainPtr dom, goto endjob; } + if (virStorageSourceIsEmpty(disk->src) || disk->src->readonly) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("can't resize empty or readonly disk '%1$s'"), + disk->dst); + goto endjob; + } + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("block resize is not supported for vhostuser disk")); @@ -9292,13 +9299,6 @@ qemuDomainBlockResize(virDomainPtr dom, disk->src->format == VIR_STORAGE_FILE_QED) size = VIR_ROUND_UP(size, 512); - if (virStorageSourceIsEmpty(disk->src) || disk->src->readonly) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("can't resize empty or readonly disk '%1$s'"), - disk->dst); - goto endjob; - } - if (!qemuDiskBusIsSD(disk->bus)) { nodename = qemuBlockStorageSourceGetEffectiveNodename(disk->src); } else { -- 2.43.0

VSH_ALTERNATIVE_OPTIONS takes just the name of the options instead of requiring also the getter functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/vsh.h b/tools/vsh.h index 377c5947c1..2a1be29b1c 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -551,6 +551,10 @@ void vshReadlineHistoryAdd(const char *cmd); } \ } while (0) +#define VSH_ALTERNATIVE_OPTIONS(NAME1, NAME2) \ + VSH_ALTERNATIVE_OPTIONS_EXPR(NAME1, vshCommandOptBool(cmd, NAME1), \ + NAME2, vshCommandOptBool(cmd, NAME2)) + /* Macros to help dealing with required options. */ /* VSH_REQUIRE_OPTION_EXPR: -- 2.43.0

Allow users to easily resize 'raw' images on block devices to the full capacity of the block device. Obviously this won't work on file-backed storage (filling the remaining capacity is most likely wrong) or for formats with metadata due to the overhead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 6 +++++- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 5 +++++ tools/virsh-domain.c | 10 +++++++++- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 3e7a4c6c22..ed1027e133 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1421,7 +1421,7 @@ blockresize :: - blockresize domain path size + blockresize domain path ([size] | [--capacity]) Resize a block device of domain while the domain is running, *path* specifies the absolute path of the block device; it corresponds @@ -1429,6 +1429,10 @@ to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to *domain* (see also ``domblklist`` for listing these names). +For image formats without metadata (raw) stored inside fixed-size storage (e.g. +block devices) the --capacity flag can be used to resize the device to the +full size of the backing device. + *size* is a scaled integer (see ``NOTES`` above) which defaults to KiB (blocks of 1024 bytes) if there is no suffix. You must use a suffix of "B" to get bytes (note that for historical reasons, this differs from diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1902546bb..30cce85b29 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2172,6 +2172,7 @@ int virDomainBlockPeek (virDomainPtr dom, */ typedef enum { VIR_DOMAIN_BLOCK_RESIZE_BYTES = 1 << 0, /* size in bytes instead of KiB (Since: 0.9.11) */ + VIR_DOMAIN_BLOCK_RESIZE_CAPACITY = 1 << 1, /* resize to full the capacity of the source (Since: 10.0.0) */ } virDomainBlockResizeFlags; int virDomainBlockResize (virDomainPtr dom, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 77a9682ecb..94e5672ed8 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -6388,6 +6388,11 @@ virDomainBlockPeek(virDomainPtr dom, * size. Depending on the file format, the hypervisor may round up * to the next alignment boundary. * + * If @flag contains VIR_DOMAIN_BLOCK_RESIZE_CAPACITY (since 10.0.0) the + * hypervisor will resize the guest block device to fully fill the source, + * ignoring @size. This is possible only for image formats with no metadata + * ('raw') and for source devices with limited capacity such as block devices. + * * The @disk parameter is either an unambiguous source name of the * block device (the <source file='...'/> sub-element, such as * "/path/to/image"), or (since 0.9.5) the device target shorthand diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fa9d356e15..327553f6d7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2949,9 +2949,12 @@ static const vshCmdOptDef opts_blockresize[] = { }, {.name = "size", .type = VSH_OT_INT, - .flags = VSH_OFLAG_REQ, .help = N_("New size of the block device, as scaled integer (default KiB)") }, + {.name = "capacity", + .type = VSH_OT_BOOL, + .help = N_("resize to capacity of source (block device)") + }, {.name = NULL} }; @@ -2963,6 +2966,11 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) unsigned long long size = 0; unsigned int flags = 0; + VSH_ALTERNATIVE_OPTIONS("size", "capacity"); + + if (vshCommandOptBool(cmd, "capacity")) + flags |= VIR_DOMAIN_BLOCK_RESIZE_CAPACITY; + if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < 0) return false; -- 2.43.0

On a Thursday in 2023, Peter Krempa wrote:
Allow users to easily resize 'raw' images on block devices to the full capacity of the block device. Obviously this won't work on file-backed storage (filling the remaining capacity is most likely wrong) or for formats with metadata due to the overhead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 6 +++++- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 5 +++++ tools/virsh-domain.c | 10 +++++++++- 4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fa9d356e15..327553f6d7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2949,9 +2949,12 @@ static const vshCmdOptDef opts_blockresize[] = { }, {.name = "size", .type = VSH_OT_INT, - .flags = VSH_OFLAG_REQ, .help = N_("New size of the block device, as scaled integer (default KiB)") }, + {.name = "capacity", + .type = VSH_OT_BOOL, + .help = N_("resize to capacity of source (block device)") + }, {.name = NULL} };
@@ -2963,6 +2966,11 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) unsigned long long size = 0; unsigned int flags = 0;
+ VSH_ALTERNATIVE_OPTIONS("size", "capacity"); + + if (vshCommandOptBool(cmd, "capacity")) + flags |= VIR_DOMAIN_BLOCK_RESIZE_CAPACITY; +
Below, there's some silly code that runs (or gets optimized away) even if size wasn't specified: /* Prefer the older interface of KiB. */ if (size % 1024 == 0) size /= 1024; Avoiding it for the --capacity flag would probably require separate bool variables. That would negate the need for the new macro. Jano
if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < 0) return false;
-- 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org

On Thu, Dec 14, 2023 at 13:26:35 +0100, Ján Tomko wrote:
On a Thursday in 2023, Peter Krempa wrote:
Allow users to easily resize 'raw' images on block devices to the full capacity of the block device. Obviously this won't work on file-backed storage (filling the remaining capacity is most likely wrong) or for formats with metadata due to the overhead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 6 +++++- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 5 +++++ tools/virsh-domain.c | 10 +++++++++- 4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fa9d356e15..327553f6d7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2949,9 +2949,12 @@ static const vshCmdOptDef opts_blockresize[] = { }, {.name = "size", .type = VSH_OT_INT, - .flags = VSH_OFLAG_REQ, .help = N_("New size of the block device, as scaled integer (default KiB)") }, + {.name = "capacity", + .type = VSH_OT_BOOL, + .help = N_("resize to capacity of source (block device)") + }, {.name = NULL} };
@@ -2963,6 +2966,11 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) unsigned long long size = 0; unsigned int flags = 0;
+ VSH_ALTERNATIVE_OPTIONS("size", "capacity"); + + if (vshCommandOptBool(cmd, "capacity")) + flags |= VIR_DOMAIN_BLOCK_RESIZE_CAPACITY; +
Below, there's some silly code that runs (or gets optimized away) even if size wasn't specified:
/* Prefer the older interface of KiB. */ if (size % 1024 == 0) size /= 1024;
Avoiding it for the --capacity flag would probably require separate bool variables. That would negate the need for the new macro.
You can also do: - /* Prefer the older interface of KiB. */ - if (size % 1024 == 0) - size /= 1024; - else - flags |= VIR_DOMAIN_BLOCK_RESIZE_BYTES; + if (vshCommandOptBool(cmd, "capacity")) { + flags |= VIR_DOMAIN_BLOCK_RESIZE_CAPACITY; + } else { + /* Prefer the older interface of KiB. */ + if (size % 1024 == 0) + size /= 1024; + else + flags |= VIR_DOMAIN_BLOCK_RESIZE_BYTES; + } I'll go with this version although it's a bit pointless either way.

On Thu, Dec 14, 2023 at 10:19:40AM +0100, Peter Krempa wrote:
Allow users to easily resize 'raw' images on block devices to the full capacity of the block device. Obviously this won't work on file-backed storage (filling the remaining capacity is most likely wrong) or for formats with metadata due to the overhead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 6 +++++- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 5 +++++ tools/virsh-domain.c | 10 +++++++++- 4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 3e7a4c6c22..ed1027e133 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1421,7 +1421,7 @@ blockresize
::
- blockresize domain path size + blockresize domain path ([size] | [--capacity])
Resize a block device of domain while the domain is running, *path* specifies the absolute path of the block device; it corresponds @@ -1429,6 +1429,10 @@ to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to *domain* (see also ``domblklist`` for listing these names).
+For image formats without metadata (raw) stored inside fixed-size storage (e.g. +block devices) the --capacity flag can be used to resize the device to the +full size of the backing device. + *size* is a scaled integer (see ``NOTES`` above) which defaults to KiB (blocks of 1024 bytes) if there is no suffix. You must use a suffix of "B" to get bytes (note that for historical reasons, this differs from diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1902546bb..30cce85b29 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2172,6 +2172,7 @@ int virDomainBlockPeek (virDomainPtr dom, */ typedef enum { VIR_DOMAIN_BLOCK_RESIZE_BYTES = 1 << 0, /* size in bytes instead of KiB (Since: 0.9.11) */ + VIR_DOMAIN_BLOCK_RESIZE_CAPACITY = 1 << 1, /* resize to full the capacity of the source (Since: 10.0.0) */ } virDomainBlockResizeFlags;
int virDomainBlockResize (virDomainPtr dom, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 77a9682ecb..94e5672ed8 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -6388,6 +6388,11 @@ virDomainBlockPeek(virDomainPtr dom, * size. Depending on the file format, the hypervisor may round up * to the next alignment boundary. * + * If @flag contains VIR_DOMAIN_BLOCK_RESIZE_CAPACITY (since 10.0.0) the + * hypervisor will resize the guest block device to fully fill the source, + * ignoring @size. This is possible only for image formats with no metadata + * ('raw') and for source devices with limited capacity such as block devices. + *
I think we should declare that @size *must* be set to either 0, or the underlying block device size, when VIR_DOMAIN_BLOCK_RESIZE_CAPACITY. If set to 0 then auto-discover the size. If set to non-zero then validate that it is an *exact* match for the block dev capacity, as a means of error checking. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Resizing of block-backed storage requires the user to pass the exact capacity of the device. Implement code which will query it instead so the user doesn't need to do that. Closes: https://gitlab.com/libvirt/libvirt/-/issues/449 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f719c33de..db99d471d4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9246,7 +9246,8 @@ qemuDomainBlockResize(virDomainPtr dom, const char *nodename = NULL; virDomainDiskDef *disk = NULL; - virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1); + virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES | + VIR_DOMAIN_BLOCK_RESIZE_CAPACITY, -1); /* We prefer operating on bytes. */ if ((flags & VIR_DOMAIN_BLOCK_RESIZE_BYTES) == 0) { @@ -9292,6 +9293,25 @@ qemuDomainBlockResize(virDomainPtr dom, goto endjob; } + if (flags & VIR_DOMAIN_BLOCK_RESIZE_CAPACITY) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); + + if (!qemuBlockStorageSourceIsRaw(disk->src) || + !virStorageSourceIsBlockLocal(disk->src)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("block resize to full capacity supported only with 'raw' local block-based disks")); + goto endjob; + } + + if (qemuDomainStorageUpdatePhysical(cfg, vm, disk->src) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to update capacity of '%1$s'"), disk->src->path); + goto endjob; + } + + size = disk->src->physical; + } + /* qcow2 and qed must be sized on 512 byte blocks/sectors, * so adjust size if necessary to round up. */ -- 2.43.0

Rather than pulling the configuration of the storage slice into the 'format' layer make the 'slice' layer effective for raw disks with a storage slice. This was made possible by the recent refactors which made the 'format' layer optional if not needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 40 +++++-------------- .../disk-slices.x86_64-latest.args | 8 ++-- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 83954690d6..fd914d2e70 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1203,27 +1203,6 @@ qemuBlockStorageSourceGetFormatLUKSProps(virStorageSource *src, } -static int -qemuBlockStorageSourceGetFormatRawProps(virStorageSource *src, - virJSONValue *props) -{ - if (virJSONValueObjectAdd(&props, "s:driver", "raw", NULL) < 0) - return -1; - - /* Currently only storage slices are supported. We'll have to calculate - * the union of the slices here if we don't want to be adding needless - * 'raw' nodes. */ - if (src->sliceStorage && - virJSONValueObjectAdd(&props, - "U:offset", src->sliceStorage->offset, - "U:size", src->sliceStorage->size, - NULL) < 0) - return -1; - - return 0; -} - - static int qemuBlockStorageSourceGetCryptoProps(virStorageSource *src, virJSONValue **encprops) @@ -1336,8 +1315,7 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSource *src) case VIR_STORAGE_FILE_FAT: /* The fat layer is emulated by the storage access layer, so we need to * put a raw layer on top */ - if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0) - return NULL; + driver = "raw"; break; case VIR_STORAGE_FILE_RAW: @@ -1345,8 +1323,7 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSource *src) if (qemuBlockStorageSourceGetFormatLUKSProps(src, props) < 0) return NULL; } else { - if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0) - return NULL; + driver = "raw"; } break; @@ -3295,10 +3272,7 @@ qemuBlockStorageSourceIsRaw(const virStorageSource *src) bool qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource *src) { - if (!src->sliceStorage) - return false; - - return !qemuBlockStorageSourceIsRaw(src); + return !!src->sliceStorage; } @@ -3314,9 +3288,13 @@ qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource *src) * existence of the format layer nodename. */ bool -qemuBlockStorageSourceNeedsFormatLayer(const virStorageSource *src G_GNUC_UNUSED) +qemuBlockStorageSourceNeedsFormatLayer(const virStorageSource *src) { - /* Currently we always create a 'format' layer */ + /* Skip 'format' layer, when a storage slice for a raw image is in use */ + if (qemuBlockStorageSourceIsRaw(src) && + src->sliceStorage) + return false; + return true; } diff --git a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args index 363f0c0361..c28eeb827e 100644 --- a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args @@ -28,8 +28,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -boot strict=on \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/raw.img","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-6-format","read-only":false,"driver":"raw","offset":0,"size":321,"file":"libvirt-6-storage"}' \ --device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-6-format","id":"virtio-disk0","bootindex":1}' \ +-blockdev '{"driver":"raw","offset":0,"size":321,"file":"libvirt-6-storage","node-name":"libvirt-6-slice-sto","read-only":false}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-6-slice-sto","id":"virtio-disk0","bootindex":1}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/raw.img","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"driver":"raw","offset":9876,"size":123456789,"file":"libvirt-5-storage","node-name":"libvirt-5-slice-sto","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"qcow2","file":"libvirt-5-slice-sto","backing":null}' \ @@ -42,8 +42,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"luks","key-secret":"libvirt-3-format-encryption-secret0","file":"libvirt-3-slice-sto"}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-3-format","id":"virtio-disk2"}' \ -blockdev '{"driver":"nvme","device":"0000:02:00.0","namespace":1,"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","offset":1234,"size":321,"file":"libvirt-2-storage"}' \ --device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x6","drive":"libvirt-2-format","id":"virtio-disk3"}' \ +-blockdev '{"driver":"raw","offset":1234,"size":321,"file":"libvirt-2-storage","node-name":"libvirt-2-slice-sto","read-only":false}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x6","drive":"libvirt-2-slice-sto","id":"virtio-disk3"}' \ -object '{"qom-type":"secret","id":"libvirt-1-format-encryption-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ -blockdev '{"driver":"nvme","device":"0001:02:00.0","namespace":2,"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \ -blockdev '{"driver":"raw","offset":1234,"size":321,"file":"libvirt-1-storage","node-name":"libvirt-1-slice-sto","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \ -- 2.43.0

Prepare the blockdev props formatter to skip formatting the slice props in case they are not applicable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 19 ++++++++++++++----- .../disk-slices.x86_64-latest.args | 10 +++++----- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index fd914d2e70..8e2da95139 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1434,22 +1434,31 @@ qemuBlockStorageSourceGetFormatProps(virStorageSource *src, * qemuBlockStorageSourceGetBlockdevStorageSliceProps: * @src: storage source object * @effective: Whether this blockdev will be the 'effective' layer of @src + * @resize: If true, the 'size' and 'offset' parameters are not formatted * * Formats the JSON object representing -blockdev configuration required to * configure a 'slice' of @src. If @effective is true, the slice layer is the - * topmost/effective blockdev layer of @src. + * topmost/effective blockdev layer of @src. If @resize is true the 'size' and + * 'offset' are not formatted, which is used to remove a slice restriction + * to resize the image. */ static virJSONValue * qemuBlockStorageSourceGetBlockdevStorageSliceProps(virStorageSource *src, - bool effective) + bool effective, + bool resize) { g_autoptr(virJSONValue) props = NULL; if (virJSONValueObjectAdd(&props, "s:driver", "raw", + "s:file", qemuBlockStorageSourceGetStorageNodename(src), + NULL) < 0) + return NULL; + + if (!resize && + virJSONValueObjectAdd(&props, "U:offset", src->sliceStorage->offset, "U:size", src->sliceStorage->size, - "s:file", qemuBlockStorageSourceGetStorageNodename(src), NULL) < 0) return NULL; @@ -1530,7 +1539,7 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSource *src, } if ((data->storageSliceNodeName = qemuBlockStorageSourceGetSliceNodename(src))) { - if (!(data->storageSliceProps = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src, effective))) + if (!(data->storageSliceProps = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src, effective, false))) return NULL; effective = false; @@ -3150,7 +3159,7 @@ qemuBlockReopenAccess(virDomainObj *vm, if (!(srcprops = qemuBlockStorageSourceGetFormatProps(src, src->backingStore))) return -1; } else if (qemuBlockStorageSourceGetSliceNodename(src)) { - if (!(srcprops = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src, true))) + if (!(srcprops = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src, true, false))) return -1; } else { if (!(srcprops = qemuBlockStorageSourceGetBackendProps(src, diff --git a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args index c28eeb827e..658391c477 100644 --- a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args @@ -28,25 +28,25 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -boot strict=on \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/raw.img","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"driver":"raw","offset":0,"size":321,"file":"libvirt-6-storage","node-name":"libvirt-6-slice-sto","read-only":false}' \ +-blockdev '{"driver":"raw","file":"libvirt-6-storage","offset":0,"size":321,"node-name":"libvirt-6-slice-sto","read-only":false}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-6-slice-sto","id":"virtio-disk0","bootindex":1}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/raw.img","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"driver":"raw","offset":9876,"size":123456789,"file":"libvirt-5-storage","node-name":"libvirt-5-slice-sto","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"driver":"raw","file":"libvirt-5-storage","offset":9876,"size":123456789,"node-name":"libvirt-5-slice-sto","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"qcow2","file":"libvirt-5-slice-sto","backing":null}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/overlay.qcow2","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"qcow2","file":"libvirt-4-storage","backing":"libvirt-5-format"}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-4-format","id":"virtio-disk1"}' \ -object '{"qom-type":"secret","id":"libvirt-3-format-encryption-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/luks.img","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"driver":"raw","offset":1234,"size":321,"file":"libvirt-3-storage","node-name":"libvirt-3-slice-sto","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"driver":"raw","file":"libvirt-3-storage","offset":1234,"size":321,"node-name":"libvirt-3-slice-sto","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"luks","key-secret":"libvirt-3-format-encryption-secret0","file":"libvirt-3-slice-sto"}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-3-format","id":"virtio-disk2"}' \ -blockdev '{"driver":"nvme","device":"0000:02:00.0","namespace":1,"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"driver":"raw","offset":1234,"size":321,"file":"libvirt-2-storage","node-name":"libvirt-2-slice-sto","read-only":false}' \ +-blockdev '{"driver":"raw","file":"libvirt-2-storage","offset":1234,"size":321,"node-name":"libvirt-2-slice-sto","read-only":false}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x6","drive":"libvirt-2-slice-sto","id":"virtio-disk3"}' \ -object '{"qom-type":"secret","id":"libvirt-1-format-encryption-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ -blockdev '{"driver":"nvme","device":"0001:02:00.0","namespace":2,"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \ --blockdev '{"driver":"raw","offset":1234,"size":321,"file":"libvirt-1-storage","node-name":"libvirt-1-slice-sto","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \ +-blockdev '{"driver":"raw","file":"libvirt-1-storage","offset":1234,"size":321,"node-name":"libvirt-1-slice-sto","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"qcow2","encrypt":{"format":"luks","key-secret":"libvirt-1-format-encryption-secret0"},"file":"libvirt-1-slice-sto"}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x7","drive":"libvirt-1-format","id":"virtio-disk4","write-cache":"on"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -- 2.43.0

The function will be used in the code for resizing block devices with a slice. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_source_conf.c | 2 +- src/conf/storage_source_conf.h | 2 ++ src/libvirt_private.syms | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index f974a521b1..959ec5ed40 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -765,7 +765,7 @@ virStorageSourceSliceCopy(const virStorageSourceSlice *src) } -static void +void virStorageSourceSliceFree(virStorageSourceSlice *slice) { if (!slice) diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 5e7d127453..05b4bda16c 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -258,6 +258,8 @@ struct _virStorageSourceSlice { char *nodename; }; +void +virStorageSourceSliceFree(virStorageSourceSlice *slice); struct _virStorageSourceFDTuple { GObject parent; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 553b01b8c0..31c0f169c3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1161,6 +1161,7 @@ virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; virStorageSourcePrivateDataFormatRelPath; virStorageSourcePrivateDataParseRelPath; +virStorageSourceSliceFree; virStorageTypeFromString; virStorageTypeToString; -- 2.43.0

Until now resizing a disk with a storage slice would break in one of the following ways: 1) for a non-raw format, the virtual size would change, but the slice would still remain in place 2) for raw disks qemu would refuse to change the size The only reasonable scenario we want to support is a 'raw' image with 0 offset (inside a block device), where we can just drop the slice. Anything else comes from a non-standard storage setup that we don't want to touch. To facilitate the resize, we first remove the 'size' parameter in qemu thus dropping the slice and then instructing qemu to resize the disk. Resolves: https://issues.redhat.com/browse/RHEL-18782 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 65 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 4 +++ src/qemu/qemu_driver.c | 77 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8e2da95139..db620bf836 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3265,6 +3265,71 @@ qemuBlockStorageSourceIsRaw(const virStorageSource *src) } +/** + * qemuBlockReopenSliceExpand: + * @vm: domain object + * @src: storage source to reopen + * + * Reopen @src image to remove it's storage slice. Note that this currently + * works only for 'raw' disks. + * + * Note: This changes transforms the definition such that the 'raw' driver + * becomes the 'format' layer rather than the 'slice' layer, to be able + * to free the slice definition. + */ +int +qemuBlockReopenSliceExpand(virDomainObj *vm, + virStorageSource *src) +{ + g_autoptr(virJSONValue) reopenoptions = virJSONValueNewArray(); + g_autoptr(virJSONValue) srcprops = NULL; + int rc; + + /* If we are lacking the object here, qemu might have opened an image with + * a node name unknown to us */ + /* Note: This is currently dead code, as only 'raw' images are supported */ + if (!src->backingStore) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("can't reopen image with unknown presence of backing store")); + return -1; + } + + /* If there is an explicit storage slice 'raw' driver layer we need to modify that */ + if (qemuBlockStorageSourceGetSliceNodename(src)) { + /* we need to know whether the slice layer is the "effective" layer */ + bool isEffective = !qemuBlockStorageSourceGetSliceNodename(src); + + if (!(srcprops = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src, isEffective, true))) + return -1; + } else { + if (!(srcprops = qemuBlockStorageSourceGetFormatProps(src, src->backingStore))) + return -1; + } + + if (virJSONValueArrayAppend(reopenoptions, &srcprops) < 0) + return -1; + + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_NONE) < 0) + return -1; + + rc = qemuMonitorBlockdevReopen(qemuDomainGetMonitor(vm), &reopenoptions); + + qemuDomainObjExitMonitor(vm); + if (rc < 0) + return -1; + + /* transform the 'slice' raw driver into a 'format' driver so that we don't + * have to add extra code */ + if (qemuBlockStorageSourceGetSliceNodename(src)) + qemuBlockStorageSourceSetFormatNodename(src, g_strdup(qemuBlockStorageSourceGetSliceNodename(src))); + + /* get rid of the slice */ + g_clear_pointer(&src->sliceStorage, virStorageSourceSliceFree); + + return 0; +} + + /** * qemuBlockStorageSourceNeedSliceLayer: * @src: source to inspect diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 9d6167b6ef..d994520924 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -267,6 +267,10 @@ qemuBlockReopenReadOnly(virDomainObj *vm, virStorageSource *src, virDomainAsyncJob asyncJob); +int +qemuBlockReopenSliceExpand(virDomainObj *vm, + virStorageSource *src); + bool qemuBlockStorageSourceIsQEMULuks(const virStorageSource *src); bool diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index db99d471d4..1b9436da5c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9245,6 +9245,7 @@ qemuDomainBlockResize(virDomainPtr dom, g_autofree char *device = NULL; const char *nodename = NULL; virDomainDiskDef *disk = NULL; + virDomainDiskDef *persistDisk = NULL; virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES | VIR_DOMAIN_BLOCK_RESIZE_CAPACITY, -1); @@ -9293,9 +9294,23 @@ qemuDomainBlockResize(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_BLOCK_RESIZE_CAPACITY) { + /* The physical capacity is needed both when automatic sizing is requested + * and when a slice is used on top of a block device. + */ + if (virStorageSourceIsBlockLocal(disk->src) && + ((flags & VIR_DOMAIN_BLOCK_RESIZE_CAPACITY) || + disk->src->sliceStorage)) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); + if (qemuDomainStorageUpdatePhysical(cfg, vm, disk->src) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to update capacity of '%1$s'"), disk->src->path); + goto endjob; + } + + } + + if (flags & VIR_DOMAIN_BLOCK_RESIZE_CAPACITY) { if (!qemuBlockStorageSourceIsRaw(disk->src) || !virStorageSourceIsBlockLocal(disk->src)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", @@ -9303,12 +9318,6 @@ qemuDomainBlockResize(virDomainPtr dom, goto endjob; } - if (qemuDomainStorageUpdatePhysical(cfg, vm, disk->src) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("failed to update capacity of '%1$s'"), disk->src->path); - goto endjob; - } - size = disk->src->physical; } @@ -9319,6 +9328,55 @@ qemuDomainBlockResize(virDomainPtr dom, disk->src->format == VIR_STORAGE_FILE_QED) size = VIR_ROUND_UP(size, 512); + if (disk->src->sliceStorage) { + if (qemuDiskBusIsSD(disk->bus)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("resize of a 'sd' disk with storage slice is not supported")); + goto endjob; + } + + /* If the storage slice has a non-zero 'offset' it's usually some weird + * configuration that we'd rather not touch */ + if (disk->src->sliceStorage->offset > 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("resize of a disk with storage slice with non-zero 'offset' is not supported")); + goto endjob; + } + + /* Removing the slice for non-raw will require introducing an attribute that + * the slice was fully expanded so that the XML can keep the 'slice' element. + * For raw images we simply remove the slice definition as there is no + * extra layer. */ + if (!qemuBlockStorageSourceIsRaw(disk->src)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("resize of a disk with storage slice is supported only for 'raw' images")); + goto endjob; + } + + /* trying to resize a block device to a size not equal to the actual + * size of the block device will cause qemu to fail */ + if (virStorageSourceIsBlockLocal(disk->src) && + disk->src->physical != size) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("block device backed disk must be resized to it's actual size '%1$llu'"), + disk->src->physical); + goto endjob; + } + + if (vm->newDef && + (persistDisk = virDomainDiskByTarget(vm->newDef, disk->dst))) { + if (!virStorageSourceIsSameLocation(disk->src, persistDisk->src) || + !persistDisk->src->sliceStorage) + persistDisk = NULL; + } + + /* remove the slice completely, we then instruct qemu to resize */ + if (qemuBlockReopenSliceExpand(vm, disk->src) < 0) + goto endjob; + + qemuDomainSaveStatus(vm); + } + if (!qemuDiskBusIsSD(disk->bus)) { nodename = qemuBlockStorageSourceGetEffectiveNodename(disk->src); } else { @@ -9333,6 +9391,11 @@ qemuDomainBlockResize(virDomainPtr dom, } qemuDomainObjExitMonitor(vm); + if (persistDisk) { + g_clear_pointer(&persistDisk->src->sliceStorage, virStorageSourceSliceFree); + qemuDomainSaveConfig(vm); + } + ret = 0; endjob: -- 2.43.0

On a Thursday in 2023, Peter Krempa wrote:
Until now resizing a disk with a storage slice would break in one of the following ways:
1) for a non-raw format, the virtual size would change, but the slice would still remain in place 2) for raw disks qemu would refuse to change the size
The only reasonable scenario we want to support is a 'raw' image with 0 offset (inside a block device), where we can just drop the slice.
Anything else comes from a non-standard storage setup that we don't want to touch.
To facilitate the resize, we first remove the 'size' parameter in qemu thus dropping the slice and then instructing qemu to resize the disk.
Resolves: https://issues.redhat.com/browse/RHEL-18782 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 65 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 4 +++ src/qemu/qemu_driver.c | 77 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 139 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8e2da95139..db620bf836 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3265,6 +3265,71 @@ qemuBlockStorageSourceIsRaw(const virStorageSource *src) }
+/** + * qemuBlockReopenSliceExpand: + * @vm: domain object + * @src: storage source to reopen + * + * Reopen @src image to remove it's storage slice. Note that this currently
*its
+ * works only for 'raw' disks. + * + * Note: This changes transforms the definition such that the 'raw' driver + * becomes the 'format' layer rather than the 'slice' layer, to be able + * to free the slice definition. + */ +int +qemuBlockReopenSliceExpand(virDomainObj *vm, + virStorageSource *src) ... @@ -9319,6 +9328,55 @@ qemuDomainBlockResize(virDomainPtr dom, ... + /* trying to resize a block device to a size not equal to the actual + * size of the block device will cause qemu to fail */ + if (virStorageSourceIsBlockLocal(disk->src) && + disk->src->physical != size) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("block device backed disk must be resized to it's actual size '%1$llu'"),
*its
+ disk->src->physical); + goto endjob; + } +
Jano

On a Thursday in 2023, Peter Krempa wrote:
This series implements two features: - automatic size discovery when resizing a raw disk on a block device - resizing of raw disks when a 'storage slice' is in use
Peter Krempa (10): qemu: block: Introduce helpers for properly testing for 'raw' and 'luks' images qemu: Use qemuBlockStorageSourceIsQEMULuks/qemuBlockStorageSourceIsRaw qemuDomainBlockResize: Agregate all checks at the beginning vsh: Introduce simple version of VSH_ALTERNATIVE_OPTIONS_EXPR virDomainBlockResize: Introduce VIR_DOMAIN_BLOCK_RESIZE_CAPACITY qemuDomainBlockResize: Implement VIR_DOMAIN_BLOCK_RESIZE_CAPACITY qemu: block: Make 'slice' layer effective for 'raw' storage source qemu: block: Format storage slice properties optionally virStorageSourceSliceFree: Export function qemuDomainBlockResize: Properly resize disks with storage slice
docs/manpages/virsh.rst | 6 +- include/libvirt/libvirt-domain.h | 1 + src/conf/storage_source_conf.c | 2 +- src/conf/storage_source_conf.h | 2 + src/libvirt-domain.c | 5 + src/libvirt_private.syms | 1 + src/qemu/qemu_block.c | 193 +++++++++++++----- src/qemu/qemu_block.h | 9 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 99 ++++++++- .../disk-slices.x86_64-latest.args | 14 +- tools/virsh-domain.c | 10 +- tools/vsh.h | 4 + 13 files changed, 276 insertions(+), 72 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Daniel P. Berrangé
-
Ján Tomko
-
Peter Krempa