From: Nikolay Ivanets <stena...@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


_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to