February 2020 - Libguestfs - libguestfs List Archives

[PATCH] lib: allow to specify physical/logical block size for disks
by Mykola Ivanets 11 Feb '20

11 Feb '20

From: Nikolay Ivanets <stenavin(a)gmail.com> Nowadays there are hard drives and operating systems which support "4K native" sector size. In this mode physical and logical block size exposed to the operating system is equal to 4096 bytes. GPT partition table (as a known example) being created in this mode will place GPT header at LBA1 which is 4096 bytes. libguetfs is unable to recognize partition table on such physical block devices or disk images. The reason is that libguestfs appliance will look for a GPT header at LBA1 which is seen at 512 byte offset. In order to fix the issue we need a way to provide correct logical block size for attached disks. Fortunately QEMU and libvirt already provides a way to specify physical/logical block size per disk basis. After discussion in a mailing list we agreed that physical block size is rarely used and is not so important. Thus both physical and logical block size will be set to the same value. In this patch one more optional parameter 'blocksize' is added to add_drive_opts API method. Valid values are 0, 512 and 4096. 0 is a special value which instructs libguestfs to do nothing special and thus we will get current behaviour. This might be used in command line tools like guestfish, virt-df and so on to reset blocksize value to a beckend-default value (similar to --format option). add_drive_scratch has the same optional parameter for a consistency and testing purpose. add-domain and add_libvirt_dom will pass logical_block_size value from libvirt XML to add_drive_opts method. --- generator/actions_core.ml | 38 ++++++++- lib/drives.c | 38 ++++++++- lib/guestfs-internal.h | 1 + lib/launch-direct.c | 25 ++++++ lib/launch-libvirt.c | 18 +++++ lib/launch-uml.c | 5 ++ lib/libvirt-domain.c | 50 ++++++++++-- tests/disks/Makefile.am | 6 +- tests/disks/test-add-drive-with-blocksize.sh | 54 +++++++++++++ tests/disks/test-qemu-drive-libvirt.xml.in | 38 ++++++++- .../test-qemu-drive-with-blocksize-libvirt.sh | 79 +++++++++++++++++++ tmp/.gitignore | 1 + 12 files changed, 339 insertions(+), 14 deletions(-) create mode 100755 tests/disks/test-add-drive-with-blocksize.sh create mode 100755 tests/disks/test-qemu-drive-with-blocksize-libvirt.sh diff --git a/generator/actions_core.ml b/generator/actions_core.ml index cb7e8dcd0..7e41e5bd0 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -210,7 +210,7 @@ this function fails and the C<errno> is set to C<EINVAL>." }; { defaults with name = "add_drive"; added = (0, 0, 3); - style = RErr, [String (PlainString, "filename")], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"]; + style = RErr, [String (PlainString, "filename")], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"; OInt "blocksize"]; once_had_no_optargs = true; blocking = false; fish_alias = ["add"]; @@ -469,6 +469,28 @@ of the same area of disk. The default is false. +=item C<blocksize> + +The integer parameter C<blocksize> allows specifying both physical and logical +block size the disk will report to the libguestfs appliance. + +Physical block size would be the value returned by the C<BLKPBSZGET> ioctl and +describes the disk's hardware sector size which can be relevant for the +alignment of disk data. + +Logical block size would be the value returned by the C<BLKSSZGET> ioctl and +describes the smallest units for disk I/O. (C<guestfs_blockdev_getss> API call +will return this value). + +Possible values for C<blocksize> parameter are 0, 512 and 4096. F<0> is a +special value which instructs libguestfs to do nothing special and it is up to +the current backend what block size to expose (usually 512 bytes). + +Only a subset of the backends support this parameter (currently only the +libvirt and direct backends do). + +The default value is 0. + =back" }; { defaults with @@ -558,6 +580,10 @@ Disks with the E<lt>readonly/E<gt> flag are skipped. =back +If present, the value of C<logical_block_size> attribute of E<lt>blockio/E<gt> +tag in libvirt XML will be passed as C<blocksize> parameter to +C<guestfs_add_drive_opts>. + The other optional parameters are passed directly through to C<guestfs_add_drive_opts>." }; @@ -597,6 +623,10 @@ The optional C<readonlydisk> parameter controls what we do for disks which are marked E<lt>readonly/E<gt> in the libvirt XML. See C<guestfs_add_domain> for possible values. +If present, the value of C<logical_block_size> attribute of E<lt>blockio/E<gt> +tag in libvirt XML will be passed as C<blocksize> parameter to +C<guestfs_add_drive_opts>. + The other optional parameters are passed directly through to C<guestfs_add_drive_opts>." }; @@ -1280,7 +1310,7 @@ function." }; { defaults with name = "add_drive_scratch"; added = (1, 23, 10); - style = RErr, [Int64 "size"], [OString "name"; OString "label"]; + style = RErr, [Int64 "size"], [OString "name"; OString "label"; OInt "blocksize"]; blocking = false; fish_alias = ["scratch"]; shortdesc = "add a temporary scratch drive"; @@ -1290,8 +1320,8 @@ C<size> parameter is the virtual size (in bytes). The scratch drive is blank initially (all reads return zeroes until you start writing to it). The drive is deleted when the handle is closed. -The optional arguments C<name> and C<label> are passed through to -C<guestfs_add_drive>." }; +The optional arguments C<name>, C<label> and C<blocksize> are passed through to +C<guestfs_add_drive_opts>." }; { defaults with name = "journal_get"; added = (1, 23, 11); diff --git a/lib/drives.c b/lib/drives.c index 5a8d29ab4..2f7ab566d 100644 --- a/lib/drives.c +++ b/lib/drives.c @@ -58,6 +58,7 @@ struct drive_create_data { const char *cachemode; enum discard discard; bool copyonread; + int blocksize; }; COMPILE_REGEXP (re_hostname_port, "(.*):(\\d+)$", 0) @@ -114,6 +115,7 @@ create_drive_file (guestfs_h *g, drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL; drv->discard = data->discard; drv->copyonread = data->copyonread; + drv->blocksize = data->blocksize; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -150,6 +152,7 @@ create_drive_non_file (guestfs_h *g, drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL; drv->discard = data->discard; drv->copyonread = data->copyonread; + drv->blocksize = data->blocksize; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -501,8 +504,13 @@ guestfs_int_drive_protocol_to_string (enum drive_protocol protocol) static char * drive_to_string (guestfs_h *g, const struct drive *drv) { + CLEANUP_FREE char *s_blocksize = NULL; + + if (drv->blocksize) + s_blocksize = safe_asprintf (g, "%d", drv->blocksize); + return safe_asprintf - (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s%s", + (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s%s%s%s", drv->src.u.path, drv->readonly ? " readonly" : "", drv->src.format ? " format=" : "", @@ -518,7 +526,9 @@ drive_to_string (guestfs_h *g, const struct drive *drv) drv->cachemode ? : "", drv->discard == discard_disable ? "" : drv->discard == discard_enable ? " discard=enable" : " discard=besteffort", - drv->copyonread ? " copyonread" : ""); + drv->copyonread ? " copyonread" : "", + drv->blocksize ? " blocksize=" : "", + drv->blocksize ? s_blocksize : ""); } /** @@ -618,6 +628,17 @@ valid_port (int port) return 1; } +/** + * Check the block size is reasonable. It can't be other then 0, 512 or 4096. + */ +static int +valid_blocksize (int blocksize) +{ + if (blocksize == 0 || blocksize == 512 || blocksize == 4096) + return 1; + return 0; +} + static int parse_one_server (guestfs_h *g, const char *server, struct drive_server *ret) { @@ -767,6 +788,10 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename, optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_COPYONREAD_BITMASK ? optargs->copyonread : false; + data.blocksize = + optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK + ? optargs->blocksize : 0; + if (data.readonly && data.discard == discard_enable) { error (g, _("discard support cannot be enabled on read-only drives")); free_drive_servers (data.servers, data.nr_servers); @@ -796,6 +821,11 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename, free_drive_servers (data.servers, data.nr_servers); return -1; } + if (!valid_blocksize (data.blocksize)) { + error (g, _("%s parameter is invalid"), "blocksize"); + free_drive_servers (data.servers, data.nr_servers); + return -1; + } if (STREQ (protocol, "file")) { if (data.servers != NULL) { @@ -982,6 +1012,10 @@ guestfs_impl_add_drive_scratch (guestfs_h *g, int64_t size, add_drive_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_LABEL_BITMASK; add_drive_optargs.label = optargs->label; } + if (optargs->bitmask & GUESTFS_ADD_DRIVE_SCRATCH_BLOCKSIZE_BITMASK) { + add_drive_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; + add_drive_optargs.blocksize = optargs->blocksize; + } /* Create the temporary file. We don't have to worry about cleanup * because everything in g->tmpdir is 'rm -rf'd when the handle is diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index 6799c265d..8c6affa54 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -261,6 +261,7 @@ struct drive { char *cachemode; enum discard discard; bool copyonread; + int blocksize; }; /* Extra hv parameters (from guestfs_config). */ diff --git a/lib/launch-direct.c b/lib/launch-direct.c index ae6ca093b..0f4bbf15f 100644 --- a/lib/launch-direct.c +++ b/lib/launch-direct.c @@ -273,6 +273,27 @@ add_drive_standard_params (guestfs_h *g, struct backend_direct_data *data, return -1; } +/** + * Add the physical_block_size and logical_block_size elements of the C<-device> + * parameter. + */ +static int +add_device_blocksize_params (guestfs_h *g, struct qemuopts *qopts, + struct drive *drv) +{ + if (drv->blocksize) { + append_list_format ("physical_block_size=%d", drv->blocksize); + append_list_format ("logical_block_size=%d", drv->blocksize); + } + + return 0; + + /* This label is called implicitly from the qemuopts macros on error. */ + qemuopts_error: + perrorf (g, "qemuopts"); + return -1; +} + static int add_drive (guestfs_h *g, struct backend_direct_data *data, struct qemuopts *qopts, size_t i, struct drive *drv) @@ -291,6 +312,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data, append_list_format ("drive=hd%zu", i); if (drv->disk_label) append_list_format ("serial=%s", drv->disk_label); + if (add_device_blocksize_params (g, qopts, drv) == -1) + return -1; } end_list (); } #if defined(__arm__) || defined(__aarch64__) || defined(__powerpc__) @@ -317,6 +340,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data, append_list_format ("drive=hd%zu", i); if (drv->disk_label) append_list_format ("serial=%s", drv->disk_label); + if (add_device_blocksize_params (g, qopts, drv) == -1) + return -1; } end_list (); } diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c index f2cad9300..49786fdd9 100644 --- a/lib/launch-libvirt.c +++ b/lib/launch-libvirt.c @@ -1043,6 +1043,7 @@ static int construct_libvirt_xml_disk (guestfs_h *g, const struct backend_libvir static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, const struct backend_libvirt_data *data, struct drive *drv, xmlTextWriterPtr xo, const char *format, const char *cachemode, enum discard discard, bool copyonread); static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); +static int construct_libvirt_xml_disk_blockio (guestfs_h *g, xmlTextWriterPtr xo, int blocksize); static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, const struct drive_source *src); static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo); static int construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo); @@ -1578,6 +1579,9 @@ construct_libvirt_xml_disk (guestfs_h *g, if (construct_libvirt_xml_disk_address (g, xo, drv_index) == -1) return -1; + if (construct_libvirt_xml_disk_blockio (g, xo, drv->blocksize) == -1) + return -1; + } end_element (); /* </disk> */ return 0; @@ -1685,6 +1689,20 @@ construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, return 0; } +static int +construct_libvirt_xml_disk_blockio (guestfs_h *g, xmlTextWriterPtr xo, + int blocksize) +{ + if (blocksize) { + start_element ("blockio") { + attribute_format ("physical_block_size", "%d", blocksize); + attribute_format ("logical_block_size", "%d", blocksize); + } end_element (); + } + + return 0; +} + static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, diff --git a/lib/launch-uml.c b/lib/launch-uml.c index da20c17d9..cd181b4b6 100644 --- a/lib/launch-uml.c +++ b/lib/launch-uml.c @@ -124,6 +124,11 @@ uml_supported (guestfs_h *g) _("uml backend does not support drives with ‘discard’ parameter set to ‘enable’")); return false; } + if (drv->blocksize) { + error (g, + _("uml backend does not support drives with ‘blocksize’ parameter")); + return false; + } } return true; diff --git a/lib/libvirt-domain.c b/lib/libvirt-domain.c index 37c0b49b2..009a22ad6 100644 --- a/lib/libvirt-domain.c +++ b/lib/libvirt-domain.c @@ -42,11 +42,12 @@ #if defined(HAVE_LIBVIRT) static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom); -static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, void *data), void *data); +static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, int blocksize, void *data), void *data); static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn); static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char *pool_nane, const char *volume_name); static bool xpath_object_is_empty (xmlXPathObjectPtr obj); static char *xpath_object_get_string (xmlDocPtr doc, xmlXPathObjectPtr obj); +static int xpath_object_get_int (xmlDocPtr doc, xmlXPathObjectPtr obj); static void ignore_errors (void *ignore, virErrorPtr ignore2) @@ -169,7 +170,7 @@ guestfs_impl_add_domain (guestfs_h *g, const char *domain_name, return r; } -static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, void *data); +static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, int blocksize, void *data); static int connect_live (guestfs_h *g, virDomainPtr dom); enum readonlydisk { @@ -331,7 +332,7 @@ static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly_in_xml, const char *protocol, char *const *server, const char *username, - const char *secret, void *datavp) + const char *secret, int blocksize, void *datavp) { struct add_disk_data *data = datavp; /* Copy whole struct so we can make local changes: */ @@ -392,6 +393,10 @@ add_disk (guestfs_h *g, optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; optargs.secret = secret; } + if (blocksize) { + optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; + optargs.blocksize = blocksize; + } return guestfs_add_drive_opts_argv (g, filename, &optargs); } @@ -486,7 +491,7 @@ for_each_disk (guestfs_h *g, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, - void *data), + int blocksize, void *data), void *data) { size_t i, nr_added = 0, nr_nodes; @@ -526,6 +531,7 @@ for_each_disk (guestfs_h *g, CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppool = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL; int readonly; + int blocksize = 0; int t; virErrorPtr err; @@ -778,8 +784,17 @@ for_each_disk (guestfs_h *g, if (!xpath_object_is_empty (xpreadonly)) readonly = 1; + /* Get logical block size. Optional. */ + xpathCtx->node = nodes->nodeTab[i]; + xpformat = xmlXPathEvalExpression (BAD_CAST + "./blockio/@logical_block_size", + xpathCtx); + if (!xpath_object_is_empty (xpformat)) + blocksize = xpath_object_get_int (doc, xpformat); + if (f) - t = f (g, filename, format, readonly, protocol, server, username, secret, data); + t = f (g, filename, format, readonly, protocol, server, username, + secret, blocksize, data); else t = 0; @@ -975,6 +990,31 @@ xpath_object_get_string (xmlDocPtr doc, xmlXPathObjectPtr obj) return value; } +/* Get the integer value from C<obj>. + * + * C<obj> is I<required> to not be empty, i.e. that C<xpath_object_is_empty> + * is C<false>. + * + * Any parsing errors are ignored and 0 (zero) will be returned. + */ +static int +xpath_object_get_int (xmlDocPtr doc, xmlXPathObjectPtr obj) +{ + xmlAttrPtr attr; + CLEANUP_FREE char *str; + int value; + + assert (obj->nodesetval->nodeTab[0]); + assert (obj->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); + attr = (xmlAttrPtr) obj->nodesetval->nodeTab[0]; + str = (char *) xmlNodeListGetString (doc, attr->children, 1); + + if (sscanf (str, "%d", &value) != 1) + value = 0; /* ignore any parsing error */ + + return value; +} + #else /* no libvirt at compile time */ #define NOT_IMPL(r) \ diff --git a/tests/disks/Makefile.am b/tests/disks/Makefile.am index 779871aff..bdbcccf5e 100644 --- a/tests/disks/Makefile.am +++ b/tests/disks/Makefile.am @@ -25,13 +25,15 @@ TESTS = \ if HAVE_LIBVIRT TESTS += \ - test-qemu-drive-libvirt.sh + test-qemu-drive-libvirt.sh \ + test-qemu-drive-with-blocksize-libvirt.sh if ENABLE_APPLIANCE TESTS += \ test-27-disks.sh \ test-255-disks.sh \ - test-add-lots-of-disks.sh + test-add-lots-of-disks.sh \ + test-add-drive-with-blocksize.sh endif endif diff --git a/tests/disks/test-add-drive-with-blocksize.sh b/tests/disks/test-add-drive-with-blocksize.sh new file mode 100755 index 000000000..a82dfbca7 --- /dev/null +++ b/tests/disks/test-add-drive-with-blocksize.sh @@ -0,0 +1,54 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2020 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Test blocksize parameter of add-drive command + +set -e + +$TEST_FUNCTIONS +skip_if_skipped + +# Test valid values +for expected_bs in 0 512 4096; do + actual_bs=$(guestfish --ro add /dev/null blocksize:${expected_bs} : run : blockdev-getss /dev/sda) + if [ ${expected_bs} -eq 0 ]; then + expected_bs=512 + fi + + if [ "${actual_bs}" != "${expected_bs}" ]; then + echo "$0: error: actual blocksize doesn't match expected: ${actual_bs} != ${expected_bs}" + exit 1 + fi +done + +# Test without blocksize parameter +expected_bs=512 +actual_bs=$(guestfish --ro add /dev/null : run : blockdev-getss /dev/sda) + +if [ "${actual_bs}" != "${expected_bs}" ]; then + echo "$0: error: actual blocksize doesn't match expected: ${actual_bs} != ${expected_bs}" + exit 1 +fi + +# Negative tests +for blocksize in 256 1000 2048 8192 65536; do + if guestfish --ro add /dev/null blocksize:${blocksize}; then + echo "$0: error: blocksize:${blocksize} should not be supported" + exit 1 + fi +done diff --git a/tests/disks/test-qemu-drive-libvirt.xml.in b/tests/disks/test-qemu-drive-libvirt.xml.in index 9b729894d..9e3eec3be 100644 --- a/tests/disks/test-qemu-drive-libvirt.xml.in +++ b/tests/disks/test-qemu-drive-libvirt.xml.in @@ -150,6 +150,43 @@ </devices> </domain> + <domain type='test' xmlns:test='http://libvirt.org/schemas/domain/test/1.0'> + <test:runstate>5</test:runstate>  + <name>blocksize</name> + <memory>1048576</memory> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/sda'/> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/sdb'/> + <blockio logical_block_size='512'/> + <target dev='vdb' bus='virtio'/> + </disk> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/sdc'/> + <blockio logical_block_size='4096'/> + <target dev='vdc' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu'/> + <source protocol='nbd'> + <host name='1.2.3.4' port='1234'/> + </source> + <blockio physical_block_size='4096' logical_block_size='512'/> + <target dev='vdd' bus='virtio'/> + </disk> + </devices> + </domain> + <pool type='dir'> <name>pool1</name> <uuid>12345678-1234-1234-1234-1234567890ab</uuid> @@ -167,7 +204,6 @@ <path>@abs_builddir@/tmp/in-pool</path> </target> </volume> - </pool> </node> diff --git a/tests/disks/test-qemu-drive-with-blocksize-libvirt.sh b/tests/disks/test-qemu-drive-with-blocksize-libvirt.sh new file mode 100755 index 000000000..1426a9e20 --- /dev/null +++ b/tests/disks/test-qemu-drive-with-blocksize-libvirt.sh @@ -0,0 +1,79 @@ +#!/bin/bash +# Copyright (C) 2013-2019 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Test that disks with <blockio .../> tag map to the correct qemu -device +# parameters and respect to logical_block_size value. + +set -e + +$TEST_FUNCTIONS +skip_if_skipped +skip_unless_libvirt_minimum_version 1 1 3 + +guestfish="guestfish -c test://$abs_builddir/test-qemu-drive-libvirt.xml" + +export LIBGUESTFS_BACKEND=direct +export LIBGUESTFS_HV="${abs_srcdir}/debug-qemu.sh" +export DEBUG_QEMU_FILE="${abs_builddir}/test-qemu-drive-with-blocksize-libvirt.out" + +function check_output () +{ + if [ ! -f "$DEBUG_QEMU_FILE" ]; then + echo "$0: guestfish command failed, see previous error messages" + exit 1 + fi +} + +function fail () +{ + echo "$0: Test $1 failed. Command line output was:" + cat "$DEBUG_QEMU_FILE" + exit 1 +} + +# arg1 - is device number +function find_device() +{ + grep -shoe "-device \S*drive=hd${1}\S*" "$DEBUG_QEMU_FILE" +} + +# arg1 - is device number +# arg2 - is expected blocksize +function check_blocksize_for_device() +{ + find_device ${1} | grep -sqEe "((physical|logical)_block_size=${2},?){2}" || fail hd${1} +} + +rm -f "$DEBUG_QEMU_FILE" + +LIBGUESTFS_DEBUG=1 $guestfish -d blocksize run ||: +check_output + +# hd0 without explicitly specified physical/logical block size. +# We don't expect neither physical_ nor logical_block_size parameter. +find_device 0 | grep -sqhve '_block_size' || fail hd0 + +# hd1 with logical_block_size='512'. +check_blocksize_for_device 1 512 + +# hd2 with logical_block_size='4096'. +check_blocksize_for_device 2 4096 + +# hd3 with physical_block_size='4096' logical_block_size='512'. +check_blocksize_for_device 3 512 + +rm -f "$DEBUG_QEMU_FILE" diff --git a/tmp/.gitignore b/tmp/.gitignore index 912a946b6..ff989cc68 100644 --- a/tmp/.gitignore +++ b/tmp/.gitignore @@ -1,6 +1,7 @@ /.guestfs-* /guestfs.* /libguestfs??????/ +/testdisk?????? /run-* /v2vovl*.qcow2 /valgrind-*.log -- 2.17.2

2 1

[nbdkit PATCH] eval: Allow user override of 'missing'
by Eric Blake 10 Feb '20

10 Feb '20

A comment in the code mentioned something that didn't actually work, but which can be useful for user-directed logging of what other callbacks they might want to implement. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I haven't pushed this one, becuase I'm not sure if we want it; but it was easy enough to whip together after an IRC question earlier today. plugins/eval/eval.c | 16 +++++++++++++--- plugins/eval/nbdkit-eval-plugin.pod | 6 ++++++ tests/test-eval.sh | 2 ++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c index 8f1eb6c..094cac5 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018-2019 Red Hat Inc. + * Copyright (C) 2018-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -169,6 +169,12 @@ create_script (const char *method, const char *value) return NULL; } + /* Special case for user override of missing */ + if (missing && strcmp (script, missing) == 0 && unlink (script) == -1) { + nbdkit_error ("unlink: %m"); + return NULL; + } + fp = fopen (script, "w"); if (fp == NULL) { nbdkit_error ("fopen: %s: %m", script); @@ -216,7 +222,7 @@ eval_load (void) /* To make things easier, create a "missing" script which always * exits with code 2. If a method is missing we call this script - * instead. It could even be overridden by the user. + * instead. It can even be overridden by the user. */ missing = create_script ("missing", "exit 2\n"); if (!missing) @@ -255,11 +261,15 @@ static int add_method (const char *key, const char *value) { char *script; + char *tmp = missing; /* Needed to allow user override of missing */ - if (get_script (key) != missing) { + missing = NULL; + if (get_script (key) != NULL) { + missing = tmp; nbdkit_error ("method %s defined more than once on the command line", key); return -1; } + missing = tmp; /* Do a bit of checking to make sure the key isn't malicious. This * duplicates work already done by nbdkit, but better safe than diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index 88c1488..cbb4133 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -140,6 +140,12 @@ no longer see that key. All of these parameters are optional. +=item B<missing=>SCRIPT + +The parameter C<missing> defines a script that will be called in place +of any other callback not explicitly provided. If omitted, this +defaults to the script "exit 2". + =back =head1 ENVIRONMENT VARIABLES diff --git a/tests/test-eval.sh b/tests/test-eval.sh index 206c680..4557b02 100755 --- a/tests/test-eval.sh +++ b/tests/test-eval.sh @@ -42,7 +42,9 @@ cleanup_fn rm -f $files nbdkit -U - eval \ get_size='echo 64M' \ pread='dd if=/dev/zero count=$3 iflag=count_bytes' \ + missing='echo "in missing: $@" >> eval.out; exit 2' \ --run 'qemu-img info $nbd' > eval.out cat eval.out grep '67108864 bytes' eval.out +grep 'in missing' eval.out -- 2.24.1

2 3

[nbdkit PATCH] ocaml: Support .preconnect callback
by Eric Blake 10 Feb '20

10 Feb '20

Somewhat of a mishmash between .open (in that it takes a bool readonly parameter) and .config_complete (in that the C code returns an int, but the Ocaml code either throws an exception or completes with unit). I did not spot any existing testsuite coverage to modify for this, and am relying on the fact that it compiles cleanly. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- plugins/ocaml/NBDKit.ml | 12 ++++++++++-- plugins/ocaml/NBDKit.mli | 4 +++- plugins/ocaml/ocaml.c | 27 ++++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml index 7002ac0..85c30a1 100644 --- a/plugins/ocaml/NBDKit.ml +++ b/plugins/ocaml/NBDKit.ml @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* nbdkit OCaml interface - * Copyright (C) 2014-2019 Red Hat Inc. + * Copyright (C) 2014-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -98,6 +98,8 @@ type 'a plugin = { thread_model : (unit -> thread_model) option; can_fast_zero : ('a -> bool) option; + + preconnect : (bool -> unit) option; } let default_callbacks = { @@ -145,6 +147,8 @@ let default_callbacks = { thread_model = None; can_fast_zero = None; + + preconnect = None; } external set_name : string -> unit = "ocaml_nbdkit_set_name" "noalloc" @@ -192,6 +196,8 @@ external set_thread_model : (unit -> thread_model) -> unit = "ocaml_nbdkit_set_t external set_can_fast_zero : ('a -> bool) -> unit = "ocaml_nbdkit_set_can_fast_zero" +external set_preconnect : (bool -> unit) -> unit = "ocaml_nbdkit_set_preconnect" + let may f = function None -> () | Some a -> f a let register_plugin plugin = @@ -257,7 +263,9 @@ let register_plugin plugin = may set_thread_model plugin.thread_model; - may set_can_fast_zero plugin.can_fast_zero + may set_can_fast_zero plugin.can_fast_zero; + + may set_preconnect plugin.preconnect external _set_error : int -> unit = "ocaml_nbdkit_set_error" "noalloc" diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli index 06648b7..4cdf911 100644 --- a/plugins/ocaml/NBDKit.mli +++ b/plugins/ocaml/NBDKit.mli @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* nbdkit OCaml interface - * Copyright (C) 2014-2019 Red Hat Inc. + * Copyright (C) 2014-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -103,6 +103,8 @@ type 'a plugin = { thread_model : (unit -> thread_model) option; can_fast_zero : ('a -> bool) option; + + preconnect : (bool -> unit) option; } (** The plugin fields and callbacks. ['a] is the handle type. *) diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c index cb69290..a7d188f 100644 --- a/plugins/ocaml/ocaml.c +++ b/plugins/ocaml/ocaml.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2014-2019 Red Hat Inc. + * Copyright (C) 2014-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -136,6 +136,8 @@ static value thread_model_fn; static value can_fast_zero_fn; +static value preconnect_fn; + /*----------------------------------------------------------------------*/ /* Wrapper functions that translate calls from C (ie. nbdkit) to OCaml. */ @@ -726,6 +728,25 @@ can_fast_zero_wrapper (void *h) CAMLreturnT (int, Bool_val (rv)); } +static int +preconnect_wrapper (int readonly) +{ + CAMLparam0 (); + CAMLlocal1 (rv); + + caml_leave_blocking_section (); + + rv = caml_callback_exn (preconnect_fn, Val_bool (readonly)); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (int, -1); + } + + caml_enter_blocking_section (); + CAMLreturnT (int, 1); +} + /*----------------------------------------------------------------------*/ /* set_* functions called from OCaml code at load time to initialize * fields in the plugin struct. @@ -815,6 +836,8 @@ SET(thread_model) SET(can_fast_zero) +SET(preconnect) + #undef SET static void @@ -861,6 +884,8 @@ remove_roots (void) REMOVE (can_fast_zero); + REMOVE (preconnect); + #undef REMOVE } -- 2.24.1

2 1

[nbdkit PATCH] split: Add support for .extents
by Eric Blake 10 Feb '20

10 Feb '20

Copies somewhat from the file plugin, with the difference that we always provide an .extents even if one or more of the split files does not support SEEK_HOLE. Testing is possible on a file system that supports sparse files when using nbdsh. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I'm going ahead and committing this, but am also posting this for review in case we later find any problems with what I just pushed. plugins/split/Makefile.am | 6 +- plugins/split/split.c | 125 +++++++++++++++++++++++++++++++++++- tests/Makefile.am | 3 + tests/test-split-extents.sh | 74 +++++++++++++++++++++ 4 files changed, 206 insertions(+), 2 deletions(-) create mode 100755 tests/test-split-extents.sh diff --git a/plugins/split/Makefile.am b/plugins/split/Makefile.am index 003f6d9..ceff349 100644 --- a/plugins/split/Makefile.am +++ b/plugins/split/Makefile.am @@ -1,5 +1,5 @@ # nbdkit -# Copyright (C) 2017 Red Hat Inc. +# Copyright (C) 2017-2020 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -42,12 +42,16 @@ nbdkit_split_plugin_la_SOURCES = \ nbdkit_split_plugin_la_CPPFLAGS = \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/utils \ $(NULL) nbdkit_split_plugin_la_CFLAGS = $(WARNINGS_CFLAGS) nbdkit_split_plugin_la_LDFLAGS = \ -module -avoid-version -shared \ -Wl,--version-script=$(top_srcdir)/plugins/plugins.syms \ $(NULL) +nbdkit_split_plugin_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(NULL) if HAVE_POD diff --git a/plugins/split/split.c b/plugins/split/split.c index 68682b2..70fd4d4 100644 --- a/plugins/split/split.c +++ b/plugins/split/split.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2017-2019 Red Hat Inc. + * Copyright (C) 2017-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -42,13 +42,19 @@ #include <errno.h> #include <sys/types.h> #include <sys/stat.h> +#include <stdbool.h> #include <nbdkit-plugin.h> +#include "cleanup.h" + /* The files. */ static char **filenames = NULL; static size_t nr_files = 0; +/* Any callbacks using lseek must be protected by this lock. */ +static pthread_mutex_t lseek_lock = PTHREAD_MUTEX_INITIALIZER; + static void split_unload (void) { @@ -96,6 +102,7 @@ struct handle { struct file { uint64_t offset, size; int fd; + bool can_extents; }; /* Create the per-connection handle. */ @@ -107,6 +114,7 @@ split_open (int readonly) size_t i; uint64_t offset; struct stat statbuf; + off_t r; h = malloc (sizeof *h); if (h == NULL) { @@ -151,6 +159,20 @@ split_open (int readonly) nbdkit_debug ("file[%zu]=%s: offset=%" PRIu64 ", size=%" PRIu64, i, filenames[i], h->files[i].offset, h->files[i].size); + +#ifdef SEEK_HOLE + /* Test if this file supports extents. */ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); + r = lseek (h->files[i].fd, 0, SEEK_DATA); + if (r == -1 && errno != ENXIO) { + nbdkit_debug ("disabling extents: lseek on %s: %m", filenames[i]); + h->files[i].can_extents = false; + } + else + h->files[i].can_extents = true; +#else + h->files[i].can_extents = false; +#endif } h->size = offset; nbdkit_debug ("total size=%" PRIu64, h->size); @@ -319,6 +341,104 @@ split_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags) } #endif /* HAVE_POSIX_FADVISE */ +#ifdef SEEK_HOLE +static int64_t +do_extents (struct file *file, uint32_t count, uint64_t offset, + bool req_one, struct nbdkit_extents *extents) +{ + int64_t r = 0; + uint64_t end = offset + count; + + do { + off_t pos; + + pos = lseek (file->fd, offset, SEEK_DATA); + if (pos == -1) { + if (errno == ENXIO) { + /* The current man page does not describe this situation well, + * but a proposed change to POSIX adds these words for ENXIO: + * "or the whence argument is SEEK_DATA and the offset falls + * within the final hole of the file." + */ + pos = end; + } + else { + nbdkit_error ("lseek: SEEK_DATA: %" PRIu64 ": %m", offset); + return -1; + } + } + + /* We know there is a hole from offset to pos-1. */ + if (pos > offset) { + if (nbdkit_add_extent (extents, offset + file->offset, pos - offset, + NBDKIT_EXTENT_HOLE | NBDKIT_EXTENT_ZERO) == -1) + return -1; + r += pos - offset; + if (req_one) + break; + } + + offset = pos; + if (offset >= end) + break; + + pos = lseek (file->fd, offset, SEEK_HOLE); + if (pos == -1) { + nbdkit_error ("lseek: SEEK_HOLE: %" PRIu64 ": %m", offset); + return -1; + } + + /* We know there is data from offset to pos-1. */ + if (pos > offset) { + if (nbdkit_add_extent (extents, offset + file->offset, pos - offset, + 0 /* allocated data */) == -1) + return -1; + r += pos - offset; + if (req_one) + break; + } + + offset = pos; + } while (offset < end); + + return r; +} + +static int +split_extents (void *handle, uint32_t count, uint64_t offset, + uint32_t flags, struct nbdkit_extents *extents) +{ + struct handle *h = handle; + const bool req_one = flags & NBDKIT_FLAG_REQ_ONE; + + while (count > 0) { + struct file *file = get_file (h, offset); + uint64_t foffs = offset - file->offset; + uint64_t max; + int64_t r; + + max = file->size - foffs; + if (max > count) + max = count; + + if (file->can_extents) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock); + max = r = do_extents (file, max, foffs, req_one, extents); + } + else + r = nbdkit_add_extent (extents, offset, max, 0 /* allocated data */); + if (r == -1) + return -1; + count -= max; + offset += max; + if (req_one) + break; + } + + return 0; +} +#endif /* SEEK_HOLE */ + static struct nbdkit_plugin plugin = { .name = "split", .version = PACKAGE_VERSION, @@ -334,6 +454,9 @@ static struct nbdkit_plugin plugin = { .pwrite = split_pwrite, #if HAVE_POSIX_FADVISE .cache = split_cache, +#endif +#ifdef SEEK_HOLE + .extents = split_extents, #endif /* In this plugin, errno is preserved properly along error return * paths from failed system calls. diff --git a/tests/Makefile.am b/tests/Makefile.am index 60583a0..ea6b147 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -193,6 +193,7 @@ EXTRA_DIST = \ test-sh-extents.sh \ test-single.sh \ test-single-from-file.sh \ + test-split-extents.sh \ test-start.sh \ test-random-sock.sh \ test-tls.sh \ @@ -672,6 +673,8 @@ test_split_SOURCES = test-split.c test_split_CFLAGS = $(WARNINGS_CFLAGS) $(LIBNBD_CFLAGS) test_split_LDADD = $(LIBNBD_LIBS) +TESTS += test-split-extents.sh + # ssh plugin test. if HAVE_SSH # Uses ‘disk’ so we have to make it conditional on guestfish. diff --git a/tests/test-split-extents.sh b/tests/test-split-extents.sh new file mode 100755 index 0000000..da385a4 --- /dev/null +++ b/tests/test-split-extents.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018-2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +# Concatenating sparse and data files should have observable extents. + +source ./functions.sh +set -e +set -x + +requires nbdsh --base-allocation -c 'exit(not h.supports_uri())' +requires truncate --help +requires stat --help + +files="test-split-extents.1 test-split-extents.2" +rm -f $files +cleanup_fn rm -f $files + +# Create two files, each half data and half sparse +truncate --size=512k test-split-extents.1 +if test "$(stat -c %b test-split-extents.1)" != 0; then + echo "$0: unable to create sparse file, skipping this test" + exit 77 +fi +printf %$((512*1024))d 1 >> test-split-extents.1 +printf %$((512*1024))d 1 >> test-split-extents.2 +truncate --size=1M test-split-extents.2 + +# Test the split plugin +nbdkit -v -U - split test-split-extents.1 test-split-extents.2 \ + --run 'nbdsh --base-allocation --uri $uri -c " +entries = [] +def f (metacontext, offset, e, err): + global entries + assert err.value == 0 + assert metacontext == nbd.CONTEXT_BASE_ALLOCATION + entries = e +h.block_status (2 * 1024 * 1024, 0, f) +assert entries == [ 512 * 1024, 3, + 1024 * 1024, 0, + 512 * 1024, 3 ] +entries = [] +# With req one, extents stop at file boundaries +h.block_status (1024 * 1024, 768 * 1024, f, nbd.CMD_FLAG_REQ_ONE) +assert entries == [ 256 * 1024, 0 ] + "' -- 2.24.1

2 1

[RFC] lib: allow to specify physical/logical block size for disks
by Mykola Ivanets 10 Feb '20

10 Feb '20

From: Nikolay Ivanets <stenavin(a)gmail.com> I faced with situation where libguestfs cannot recognize partitions on a disk image which was partitioned on a system with "4K native" sector size support. In order to fix the issue we need to allow users to specify desired physical and/or logical block size per drive basis. It is definitely not a complete patch but rather a way to request for a comments. Nevertheless it is already working patch. I've added an optional parameters to add_drive API method which allow specifying physical and logical block size for a drive separetely. There are no documentation and tests yet. Input parameters are not validated for correctness. Here are my questions: - Am I move in the right direction adding support for physical/logical block size? - I'm not sure I've made a good choise for parameter names: 'pblocksize' and 'lblocksize' respectively. I've tried to avoid long names like 'physicalblocksize' while keeping readability and semantic. - Do we want to add the same optional parameters to 'add_drive_scratch' API method? I think it would be nice but it is up to you. - What about 'add_dom', 'add_libvirt_dom' API methods? Should they also handle 'blokio' tag in domain XML and act respectively? - Anything else I didn't spot yet? - Do we want guestfish to accept physical/logical block size per drive from command line? - What about other virt tools like virt-df, virt-cat and so on? Sorry for a long list of questions. --- generator/actions_core.ml | 2 +- lib/drives.c | 14 ++++++++++++++ lib/guestfs-internal.h | 2 ++ lib/launch-direct.c | 24 ++++++++++++++++++++++++ lib/launch-libvirt.c | 21 +++++++++++++++++++++ lib/launch-uml.c | 10 ++++++++++ 6 files changed, 72 insertions(+), 1 deletion(-) diff --git a/generator/actions_core.ml b/generator/actions_core.ml index cb7e8dcd0..9de3a6484 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -210,7 +210,7 @@ this function fails and the C<errno> is set to C<EINVAL>." }; { defaults with name = "add_drive"; added = (0, 0, 3); - style = RErr, [String (PlainString, "filename")], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"]; + style = RErr, [String (PlainString, "filename")], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"; OInt "pblocksize"; OInt "lblocksize"]; once_had_no_optargs = true; blocking = false; fish_alias = ["add"]; diff --git a/lib/drives.c b/lib/drives.c index 5a8d29ab4..bb160cc34 100644 --- a/lib/drives.c +++ b/lib/drives.c @@ -58,6 +58,8 @@ struct drive_create_data { const char *cachemode; enum discard discard; bool copyonread; + int pblocksize; + int lblocksize; }; COMPILE_REGEXP (re_hostname_port, "(.*):(\\d+)$", 0) @@ -114,6 +116,8 @@ create_drive_file (guestfs_h *g, drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL; drv->discard = data->discard; drv->copyonread = data->copyonread; + drv->pblocksize = data->pblocksize; + drv->lblocksize = data->lblocksize; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -150,6 +154,8 @@ create_drive_non_file (guestfs_h *g, drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL; drv->discard = data->discard; drv->copyonread = data->copyonread; + drv->pblocksize = data->pblocksize; + drv->lblocksize = data->lblocksize; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -767,6 +773,14 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename, optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_COPYONREAD_BITMASK ? optargs->copyonread : false; + data.pblocksize = + optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_PBLOCKSIZE_BITMASK + ? optargs->pblocksize : 0; + + data.lblocksize = + optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_LBLOCKSIZE_BITMASK + ? optargs->lblocksize : 0; + if (data.readonly && data.discard == discard_enable) { error (g, _("discard support cannot be enabled on read-only drives")); free_drive_servers (data.servers, data.nr_servers); diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index 6799c265d..20f22a674 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -261,6 +261,8 @@ struct drive { char *cachemode; enum discard discard; bool copyonread; + int pblocksize; + int lblocksize; }; /* Extra hv parameters (from guestfs_config). */ diff --git a/lib/launch-direct.c b/lib/launch-direct.c index ae6ca093b..518bd24fc 100644 --- a/lib/launch-direct.c +++ b/lib/launch-direct.c @@ -273,6 +273,26 @@ add_drive_standard_params (guestfs_h *g, struct backend_direct_data *data, return -1; } +/** + * Add the blockio elements of the C<-device> parameter. + */ +static int +add_device_blockio_params (guestfs_h *g, struct qemuopts *qopts, + struct drive *drv) +{ + if (drv->pblocksize) + append_list_format ("physical_block_size=%d", drv->pblocksize); + if (drv->lblocksize) + append_list_format ("logical_block_size=%d", drv->lblocksize); + + return 0; + + /* This label is called implicitly from the qemuopts macros on error. */ + qemuopts_error: + perrorf (g, "qemuopts"); + return -1; +} + static int add_drive (guestfs_h *g, struct backend_direct_data *data, struct qemuopts *qopts, size_t i, struct drive *drv) @@ -291,6 +311,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data, append_list_format ("drive=hd%zu", i); if (drv->disk_label) append_list_format ("serial=%s", drv->disk_label); + if (add_device_blockio_params (g, qopts, drv) == -1) + return -1; } end_list (); } #if defined(__arm__) || defined(__aarch64__) || defined(__powerpc__) @@ -317,6 +339,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data, append_list_format ("drive=hd%zu", i); if (drv->disk_label) append_list_format ("serial=%s", drv->disk_label); + if (add_device_blockio_params (g, qopts, drv) == -1) + return -1; } end_list (); } diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c index f2cad9300..4ce2fa683 100644 --- a/lib/launch-libvirt.c +++ b/lib/launch-libvirt.c @@ -1043,6 +1043,7 @@ static int construct_libvirt_xml_disk (guestfs_h *g, const struct backend_libvir static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, const struct backend_libvirt_data *data, struct drive *drv, xmlTextWriterPtr xo, const char *format, const char *cachemode, enum discard discard, bool copyonread); static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); +static int construct_libvirt_xml_disk_blockio (guestfs_h *g, xmlTextWriterPtr xo, int pblocksize, int lblocksize); static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, const struct drive_source *src); static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo); static int construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo); @@ -1578,6 +1579,10 @@ construct_libvirt_xml_disk (guestfs_h *g, if (construct_libvirt_xml_disk_address (g, xo, drv_index) == -1) return -1; + if (construct_libvirt_xml_disk_blockio (g, xo, drv->pblocksize, + drv->lblocksize) == -1) + return -1; + } end_element (); /* </disk> */ return 0; @@ -1685,6 +1690,22 @@ construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, return 0; } +static int construct_libvirt_xml_disk_blockio (guestfs_h *g, + xmlTextWriterPtr xo, + int pblocksize, int lblocksize) +{ + if (pblocksize || lblocksize) { + start_element ("blockio") { + if (pblocksize) + attribute_format ("physical_block_size", "%d", pblocksize); + if (lblocksize) + attribute_format ("logical_block_size", "%d", lblocksize); + } end_element (); + } + + return 0; +} + static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, diff --git a/lib/launch-uml.c b/lib/launch-uml.c index da20c17d9..274287b58 100644 --- a/lib/launch-uml.c +++ b/lib/launch-uml.c @@ -124,6 +124,16 @@ uml_supported (guestfs_h *g) _("uml backend does not support drives with ‘discard’ parameter set to ‘enable’")); return false; } + if (drv->pblocksize) { + error (g, + _("uml backend does not support drives with ‘physical_block_size’ parameter")); + return false; + } + if (drv->lblocksize) { + error (g, + _("uml backend does not support drives with ‘logical_block_size’ parameter")); + return false; + } } return true; -- 2.17.2

6 14

[PATCH libnbd 0/2] api: Add support for AF_VSOCK.
by Richard W.M. Jones 10 Feb '20

10 Feb '20

This is a series of patches to libnbd and nbdkit adding AF_VSOCK support. On the host side it allows you to start an nbdkit instance which listens on a virtio-vsock socket: $ ./nbdkit -fv --vsock memory 1G ... nbdkit: debug: bound to vsock 2:10809 On the guest side you can then use libnbd to connect to the server: $ ./run nbdsh -c 'h.connect_vsock(2, 10809)' -c 'print(h.get_size())' 1073741824 $ ./run nbdfuse mp --vsock 2 10809 & $ ll mp/ total 0 -rw-rw-rw-. 1 rjones rjones 1073741824 Oct 18 16:23 nbd $ dd if=/dev/random of=mp/nbd bs=1024 count=100 conv=notrunc,nocreat dd: warning: partial read (84 bytes); suggest iflag=fullblock 0+100 records in 0+100 records out 6851 bytes (6.9 kB, 6.7 KiB) copied, 0.013797 s, 497 kB/s (Performance of FUSE is not great, it should be better using raw libnbd.) I mainly wrote this to show that it can be done. It's unclear if this would be faster or slower than the usual way that NBD devices are exposed to guests via virtio-blk/-scsi. https://wiki.qemu.org/Features/VirtioVsock Thanks: Stefan Hajnoczi for help with debugging this.

3 10

[PATCH v2] launch: add support for autodetection of appliance image format
by Pavel Butsykin 06 Feb '20

06 Feb '20

This feature allows you to use different image formats for the fixed appliance. The raw format is used by default. Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com> --- lib/launch-direct.c | 2 ++ lib/launch-libvirt.c | 19 ++++++++++++------- m4/guestfs_appliance.m4 | 11 +++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/launch-direct.c b/lib/launch-direct.c index 0be662e25..b9b54857a 100644 --- a/lib/launch-direct.c +++ b/lib/launch-direct.c @@ -592,7 +592,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) append_list ("id=appliance"); append_list ("cache=unsafe"); append_list ("if=none"); +#ifndef APPLIANCE_FMT_AUTO append_list ("format=raw"); +#endif } end_list (); start_list ("-device") { append_list ("scsi-hd"); diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c index 4adb2cfb3..030ea6911 100644 --- a/lib/launch-libvirt.c +++ b/lib/launch-libvirt.c @@ -212,9 +212,10 @@ get_source_format_or_autodetect (guestfs_h *g, struct drive *drv) /** * Create a qcow2 format overlay, with the given C<backing_drive> - * (file). The C<format> parameter, which must be non-NULL, is the - * backing file format. This is used to create the appliance overlay, - * and also for read-only drives. + * (file). The C<format> parameter is the backing file format. + * The C<format> parameter can be NULL, in this case the backing + * format will be determined automatically. This is used to create + * the appliance overlay, and also for read-only drives. */ static char * make_qcow2_overlay (guestfs_h *g, const char *backing_drive, @@ -223,8 +224,6 @@ make_qcow2_overlay (guestfs_h *g, const char *backing_drive, char *overlay; struct guestfs_disk_create_argv optargs; - assert (format != NULL); - if (guestfs_int_lazy_make_tmpdir (g) == -1) return NULL; @@ -232,8 +231,10 @@ make_qcow2_overlay (guestfs_h *g, const char *backing_drive, optargs.bitmask = GUESTFS_DISK_CREATE_BACKINGFILE_BITMASK; optargs.backingfile = backing_drive; - optargs.bitmask |= GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK; - optargs.backingformat = format; + if (format) { + optargs.bitmask |= GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK; + optargs.backingformat = format; + } if (guestfs_disk_create_argv (g, overlay, "qcow2", -1, &optargs) == -1) { free (overlay); @@ -461,7 +462,11 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) /* Note that appliance can be NULL if using the old-style appliance. */ if (appliance) { +#ifdef APPLIANCE_FMT_AUTO + params.appliance_overlay = make_qcow2_overlay (g, appliance, NULL); +#else params.appliance_overlay = make_qcow2_overlay (g, appliance, "raw"); +#endif if (!params.appliance_overlay) goto cleanup; } diff --git a/m4/guestfs_appliance.m4 b/m4/guestfs_appliance.m4 index 81c43879f..4e1ec8135 100644 --- a/m4/guestfs_appliance.m4 +++ b/m4/guestfs_appliance.m4 @@ -139,3 +139,14 @@ AC_SUBST([GUESTFS_DEFAULT_PATH]) AC_DEFINE_UNQUOTED([GUESTFS_DEFAULT_PATH], ["$GUESTFS_DEFAULT_PATH"], [Define guestfs default path.]) + +AC_ARG_ENABLE([appliance-fmt-auto], + [AS_HELP_STRING([--enable-appliance-fmt-auto], + [enable autodetection of appliance image format @<:@default=no@:>@])], + [ENABLE_APPLIANCE_FMT_AUTO="$enableval"], + [ENABLE_APPLIANCE_FMT_AUTO=no]) + +if test "x$ENABLE_APPLIANCE_FMT_AUTO" = "xyes"; then + AC_DEFINE([APPLIANCE_FMT_AUTO], [1], + [Define to 1 if enabled autodetection of appliance image format.]) +fi -- 2.13.0

2 4

[PATCH v2 0/2] Fixes and tweak to the installation of qemu-ga MSI
by Tomáš Golembiovský 06 Feb '20

06 Feb '20

This, together with the changes to common repo are fixes to the installation qemu-ga MSI. There is still an issue that I did not figure yet how to fix. On Windows 10 it fails to register the QEMU-GA service. Tomáš Golembiovský (2): windows: fix detection of qemu-ga installer on RHV windows: small tweaks of qemu-ga firstboot script v2v/convert_windows.ml | 8 +++++++- v2v/windows_virtio.ml | 5 ++--- 2 files changed, 9 insertions(+), 4 deletions(-) -- 2.25.0

2 3

[PATCH] properly initialize error_data_lock_list before use
by Daria Phoebe Brashear 06 Feb '20

06 Feb '20

Required such that macOS doesn't crash in get_error_data (via call stack from guestfs_launch) >From 5b121bc8bb8f1fadf835b4af30cbb9c9e95af258 Mon Sep 17 00:00:00 2001 From: Daria Phoebe Brashear <dariaphoebe(a)auristor.com> Date: Tue, 4 Feb 2020 20:25:10 -0500 Subject: [PATCH] libhandle: initialize error_data_list_lock when a handle is allocated, the error_data_list_lock must be initialized --- lib/handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/handle.c b/lib/handle.c index f61fdbcd3..99a8b8848 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -88,6 +88,7 @@ guestfs_create_flags (unsigned flags, ...) if (!g) return NULL; gl_recursive_lock_init (g->lock); + gl_lock_init (g->error_data_list_lock); g->state = CONFIG; @@ -176,6 +177,7 @@ guestfs_create_flags (unsigned flags, ...) free (g->append); guestfs_int_free_error_data_list (g); gl_tls_key_destroy (g->error_data); + gl_lock_destroy (g->error_data_list_lock); gl_recursive_lock_destroy (g->lock); free (g); return NULL; -- 2.21.1 (Apple Git-122.3)

2 1

[PATCH 0/2] Fixes and tweak to the installation of qemu-ga MSI
by Tomáš Golembiovský 05 Feb '20

05 Feb '20

This, together with the changes to common repo are fixes to the installation qemu-ga MSI. There is still an issue that I did not figure yet how to fix. On Windows 10 it fails to register the QEMU-GA service. Tomáš Golembiovský (2): windows: fix detection of qemu-ga installer on RHV windows: small tweaks of qemu-ga firstboot script v2v/convert_windows.ml | 8 +++++++- v2v/windows_virtio.ml | 5 ++--- 2 files changed, 9 insertions(+), 4 deletions(-) -- 2.24.0

3 8