5:25 p.m.
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