Libguestfs - Nov 2019 - Re: [PATCH] fish: add option --blocksize for disks

When --blocksize <val> is provided, qemu command line would add
physical_block_size=<val>,physical_logical_size=<val> to -device
directive.

Example:
qemu-kvm \
    -drive file=raw.img,cache=writeback,id=hd0,if=none \
    -device scsi-hd,drive=hd0,physical_block_size=4096,logical_block_size=4096 \

Signed-off-by: Tuan Hoang <tmhoang@linux.ibm.com>
---
 fish/fish.c         | 5 +++++
 generator/c.ml      | 2 ++
 lib/launch-direct.c | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/fish/fish.c b/fish/fish.c
index 2070e37..dd58d23 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -124,6 +124,7 @@ usage (int status)
               "  -h|--cmd-help        List available commands\n"
               "  -h|--cmd-help cmd    Display detailed help on
‘cmd’\n"
               "  -a|--add image       Add image\n"
+              "  --blocksize          Specify physical and logical
blocksize\n"
               "  -c|--connect uri     Specify libvirt URI for -d
option\n"
               "  --csh                Make --listen csh-compatible\n"
               "  -d|--domain guest    Add disks from libvirt guest\n"
@@ -190,6 +191,7 @@ main (int argc, char *argv[])
   static const char options[] = "a:c:d:Df:h::im:nN:rvVwx";
   static const struct option long_options[] = {
     { "add", 1, 0, 'a' },
+    { "blocksize", 1, 0, 0 },
     { "cmd-help", 2, 0, 'h' },
     { "connect", 1, 0, 'c' },
     { "csh", 0, 0, 0 },
@@ -259,6 +261,9 @@ main (int argc, char *argv[])
         display_long_options (long_options);
       else if (STREQ (long_options[option_index].name,
"short-options"))
         display_short_options (options);
+      else if (STREQ (long_options[option_index].name, "blocksize"))
{
+        blocksize = strtol(optarg, NULL, 0);
+      }
       else if (STREQ (long_options[option_index].name, "listen"))
         remote_control_listen = 1;
       else if (STREQ (long_options[option_index].name, "remote")) {
diff --git a/generator/c.ml b/generator/c.ml
index 86f7d89..61aa50d 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -439,6 +439,8 @@ extern \"C\" {
     (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__)
 #endif
 
+extern int blocksize;
+
 /* Define GUESTFS_WARN_DEPRECATED=1 to warn about deprecated API functions. */
 #define GUESTFS_DEPRECATED_NO_REPLACEMENT
 #define GUESTFS_DEPRECATED_REPLACED_BY(s)
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index ee2dcb8..57d1ad6 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -49,6 +49,8 @@
 #include "guestfs_protocol.h"
 #include "qemuopts.h"
 
+int blocksize = 0;
+
 /* Per-handle data. */
 struct backend_direct_data {
   pid_t pid;                    /* Qemu PID. */
@@ -315,6 +317,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data,
     start_list ("-device") {
       append_list ("scsi-hd");
       append_list_format ("drive=hd%zu", i);
+      append_list_format ("physical_block_size=%d", blocksize);
+      append_list_format ("logical_block_size=%d", blocksize);
       if (drv->disk_label)
         append_list_format ("serial=%s", drv->disk_label);
     } end_list ();
-- 
2.21.0

Hi Richard,

After our discussion in IRC, I agreed that we should patch generator
files and use guestfs_add_drive_opts(). That worked out well. But when
running, {physical,logical}_block_size= are added to qemu's -disk
directive instead, and it fails. Those 2 should be added to qemu's
-device directive instead.

Now current patch is more my idea than a working approach:

make[3]: Entering directory
'/data/rpmbuild/SOURCES/libguestfs-1.40.2/fish'
  CCLD     guestfish
/usr/bin/ld: guestfish-fish.o: in function `main':
/home/tmhoang/rpmbuild/SOURCES/libguestfs-1.40.2/fish/fish.c:265:
undefined reference to `blocksize'


I couldn't find a way to share a variable between `fist/fish.c` and
`lib/launch-direct.c`. Maybe you can shed some light on this.

Many thanks,
Tuan

On 10/29/19 9:39 PM, Tuan Hoang wrote:
> When --blocksize <val> is provided, qemu command line would add
> physical_block_size=<val>,physical_logical_size=<val> to
-device
> directive.
> 
> Example:
> qemu-kvm \
>     -drive file=raw.img,cache=writeback,id=hd0,if=none \
>     -device
scsi-hd,drive=hd0,physical_block_size=4096,logical_block_size=4096 \
> 
> Signed-off-by: Tuan Hoang <tmhoang@linux.ibm.com>
> ---
>  fish/fish.c         | 5 +++++
>  generator/c.ml      | 2 ++
>  lib/launch-direct.c | 4 ++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/fish/fish.c b/fish/fish.c
> index 2070e37..dd58d23 100644
> --- a/fish/fish.c
> +++ b/fish/fish.c
> @@ -124,6 +124,7 @@ usage (int status)
>                "  -h|--cmd-help        List available commands\n"
>                "  -h|--cmd-help cmd    Display detailed help on
‘cmd’\n"
>                "  -a|--add image       Add image\n"
> +              "  --blocksize          Specify physical and logical
blocksize\n"
>                "  -c|--connect uri     Specify libvirt URI for -d
option\n"
>                "  --csh                Make --listen
csh-compatible\n"
>                "  -d|--domain guest    Add disks from libvirt
guest\n"
> @@ -190,6 +191,7 @@ main (int argc, char *argv[])
>    static const char options[] = "a:c:d:Df:h::im:nN:rvVwx";
>    static const struct option long_options[] = {
>      { "add", 1, 0, 'a' },
> +    { "blocksize", 1, 0, 0 },
>      { "cmd-help", 2, 0, 'h' },
>      { "connect", 1, 0, 'c' },
>      { "csh", 0, 0, 0 },
> @@ -259,6 +261,9 @@ main (int argc, char *argv[])
>          display_long_options (long_options);
>        else if (STREQ (long_options[option_index].name,
"short-options"))
>          display_short_options (options);
> +      else if (STREQ (long_options[option_index].name,
"blocksize")) {
> +        blocksize = strtol(optarg, NULL, 0);
> +      }
>        else if (STREQ (long_options[option_index].name,
"listen"))
>          remote_control_listen = 1;
>        else if (STREQ (long_options[option_index].name,
"remote")) {
> diff --git a/generator/c.ml b/generator/c.ml
> index 86f7d89..61aa50d 100644
> --- a/generator/c.ml
> +++ b/generator/c.ml
> @@ -439,6 +439,8 @@ extern \"C\" {
>      (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__)
>  #endif
>  
> +extern int blocksize;
> +
>  /* Define GUESTFS_WARN_DEPRECATED=1 to warn about deprecated API
functions. */
>  #define GUESTFS_DEPRECATED_NO_REPLACEMENT
>  #define GUESTFS_DEPRECATED_REPLACED_BY(s)
> diff --git a/lib/launch-direct.c b/lib/launch-direct.c
> index ee2dcb8..57d1ad6 100644
> --- a/lib/launch-direct.c
> +++ b/lib/launch-direct.c
> @@ -49,6 +49,8 @@
>  #include "guestfs_protocol.h"
>  #include "qemuopts.h"
>  
> +int blocksize = 0;
> +
>  /* Per-handle data. */
>  struct backend_direct_data {
>    pid_t pid;                    /* Qemu PID. */
> @@ -315,6 +317,8 @@ add_drive (guestfs_h *g, struct backend_direct_data
*data,
>      start_list ("-device") {
>        append_list ("scsi-hd");
>        append_list_format ("drive=hd%zu", i);
> +      append_list_format ("physical_block_size=%d", blocksize);
> +      append_list_format ("logical_block_size=%d", blocksize);
>        if (drv->disk_label)
>          append_list_format ("serial=%s", drv->disk_label);
>      } end_list ();
>

I'm at the KVM Forum at the moment, and next week I'm at another
conference, so don't have time to review this patch right now.
Maybe others will comment.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW