[PATCH] [PATCH RFC v3] Implements Backend Program conventions for vhost-user-scsi

Sakshi Kaushik posted 1 patch 2 years, 1 month ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220405122238.1666-1-sakshikaushik717@gmail.com
Maintainers: Raphael Norwitz <raphael.norwitz@nutanix.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
contrib/vhost-user-scsi/vhost-user-scsi.c | 76 +++++++++++++++--------
1 file changed, 51 insertions(+), 25 deletions(-)
[PATCH] [PATCH RFC v3] Implements Backend Program conventions for vhost-user-scsi
Posted by Sakshi Kaushik 2 years, 1 month ago
Signed-off-by: Sakshi Kaushik <sakshikaushik717@gmail.com>
---
 contrib/vhost-user-scsi/vhost-user-scsi.c | 76 +++++++++++++++--------
 1 file changed, 51 insertions(+), 25 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 4f6e3e2a24..74ec44d190 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -351,34 +351,58 @@ fail:
 
 /** vhost-user-scsi **/
 
+int opt_fdnum = -1;
+char *opt_socket_path;
+gboolean opt_print_caps;
+char *iscsi_uri;
+
+static GOptionEntry entries[] = {
+    { "print-capabilities", 'c', 0, G_OPTION_ARG_NONE, &opt_print_caps,
+      "Print capabilities", NULL },
+    { "fd", 'f', 0, G_OPTION_ARG_INT, &opt_fdnum,
+      "Use inherited fd socket", "FDNUM" },
+    { "iscsi_uri", 'i', 0, G_OPTION_ARG_FILENAME, &iscsi_uri,
+      "Use inherited fd socket", "FDNUM" },
+    { "socket-path", 's', 0, G_OPTION_ARG_FILENAME, &opt_socket_path,
+      "Use UNIX socket path", "PATH" }
+};
+
 int main(int argc, char **argv)
 {
     VusDev *vdev_scsi = NULL;
-    char *unix_fn = NULL;
-    char *iscsi_uri = NULL;
-    int lsock = -1, csock = -1, opt, err = EXIT_SUCCESS;
-
-    while ((opt = getopt(argc, argv, "u:i:")) != -1) {
-        switch (opt) {
-        case 'h':
-            goto help;
-        case 'u':
-            unix_fn = g_strdup(optarg);
-            break;
-        case 'i':
-            iscsi_uri = g_strdup(optarg);
-            break;
-        default:
-            goto help;
-        }
+    int lsock = -1, csock = -1, err = EXIT_SUCCESS;
+
+    GError *error = NULL;
+    GOptionContext *context;
+
+    context = g_option_context_new(NULL);
+    g_option_context_add_main_entries(context, entries, NULL);
+    if (!g_option_context_parse(context, &argc, &argv, &error)) {
+        g_printerr("Option parsing failed: %s\n", error->message);
+        exit(EXIT_FAILURE);
     }
-    if (!unix_fn || !iscsi_uri) {
+
+    if (opt_print_caps) {
+        g_print("{\n");
+        g_print("  \"type\": \"scsi\",\n");
+        g_print("}\n");
+        goto out;
+    }
+
+    if (!opt_socket_path || !iscsi_uri) {
         goto help;
     }
 
-    lsock = unix_sock_new(unix_fn);
-    if (lsock < 0) {
-        goto err;
+    if (opt_socket_path) {
+        lsock = unix_sock_new(opt_socket_path);
+        if (lsock < 0) {
+            exit(EXIT_FAILURE);
+        }
+    } else if (opt_fdnum < 0) {
+        g_print("%s\n", g_option_context_get_help(context, true, NULL));
+        exit(EXIT_FAILURE);
+    } else {
+        lsock = opt_fdnum;
     }
 
     csock = accept(lsock, NULL, NULL);
@@ -408,7 +432,7 @@ out:
     if (vdev_scsi) {
         g_main_loop_unref(vdev_scsi->loop);
         g_free(vdev_scsi);
-        unlink(unix_fn);
+        unlink(opt_socket_path);
     }
     if (csock >= 0) {
         close(csock);
@@ -416,7 +440,7 @@ out:
     if (lsock >= 0) {
         close(lsock);
     }
-    g_free(unix_fn);
+    g_free(opt_socket_path);
     g_free(iscsi_uri);
 
     return err;
@@ -426,10 +450,12 @@ err:
     goto out;
 
 help:
-    fprintf(stderr, "Usage: %s [ -u unix_sock_path -i iscsi_uri ] | [ -h ]\n",
+    fprintf(stderr, "Usage: %s [ -s socket-path -i iscsi_uri -f fd -p print-capabilities ] | [ -h ]\n",
             argv[0]);
-    fprintf(stderr, "          -u path to unix socket\n");
+    fprintf(stderr, "          -s path to unix socket\n");
     fprintf(stderr, "          -i iscsi uri for lun 0\n");
+    fprintf(stderr, "          -f fd, file-descriptor\n");
+    fprintf(stderr, "          -p denotes print-capabilities\n");
     fprintf(stderr, "          -h print help and quit\n");
 
     goto err;
-- 
2.17.1
Re: [PATCH] [PATCH RFC v3] Implements Backend Program conventions for vhost-user-scsi
Posted by Stefan Hajnoczi 2 years, 1 month ago
On Tue, Apr 05, 2022 at 07:22:38AM -0500, Sakshi Kaushik wrote:

Thanks for the patch! Comments below:

> Signed-off-by: Sakshi Kaushik <sakshikaushik717@gmail.com>
> ---
>  contrib/vhost-user-scsi/vhost-user-scsi.c | 76 +++++++++++++++--------
>  1 file changed, 51 insertions(+), 25 deletions(-)
> 
> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
> index 4f6e3e2a24..74ec44d190 100644
> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
> @@ -351,34 +351,58 @@ fail:
>  
>  /** vhost-user-scsi **/
>  
> +int opt_fdnum = -1;
> +char *opt_socket_path;
> +gboolean opt_print_caps;
> +char *iscsi_uri;

These variables can be declared static since they don't need to be
externally visible (they aren't used by any other .c files).

> +    if (opt_print_caps) {
> +        g_print("{\n");
> +        g_print("  \"type\": \"scsi\",\n");

The JSON grammar (https://www.json.org/json-en.html) does not allow
trailing commas in objects. Please remove the comma on this line.

(Some parses may accept the trailing comma but others may reject the
input as invalid JSON.)

> @@ -426,10 +450,12 @@ err:
>      goto out;
>  
>  help:
> -    fprintf(stderr, "Usage: %s [ -u unix_sock_path -i iscsi_uri ] | [ -h ]\n",
> +    fprintf(stderr, "Usage: %s [ -s socket-path -i iscsi_uri -f fd -p print-capabilities ] | [ -h ]\n",
>              argv[0]);
> -    fprintf(stderr, "          -u path to unix socket\n");
> +    fprintf(stderr, "          -s path to unix socket\n");
>      fprintf(stderr, "          -i iscsi uri for lun 0\n");
> +    fprintf(stderr, "          -f fd, file-descriptor\n");
> +    fprintf(stderr, "          -p denotes print-capabilities\n");
>      fprintf(stderr, "          -h print help and quit\n");

The vhost-user backend conventions only document the long options
("--socket-path", "--fd", "--print-capabilities"). It's okay to offer
short options too but please show the long options in the help output
since it's the standard interface and that will make it clear that this
program supports the vhost-user backend program conventions.

Something like:

  -s, --socket-path=SOCKET_PATH     path to unix socket