From: Nikolay Ivanets <stenavin(a)gmail.com&gt; 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