[PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Jorge Lucángeli Obes]

[-- Attachment #1: Type: text/plain, Size: 6626 bytes --]

Hi all,

The last time this issue was discussed, the executable-directory idea
gained more consensus than the snapshot-based idea. This patch
implements my perception of the first idea. Non-Windows for now, as I
can't test on a Windows system. Suggestions and constructive criticism
more than welcome.

Cheers,
Jorge

This patch allows QEMU to execute a directory with a special format.

This patch allows storing command line options in a configuration file inside
a directory and then directly executing that directory. A simple check is
included to prevent the configuration file to access image files outside
the executed directory. Extra command line options can be passed on invocation,
which will take precedence over the ones stored in the configuration file.

The configuration file specifies, on its first line, the image file to use.
The rest of the file specifies command line options separated by spaces or
newlines. Careful reconstruction of the command line makes sure the speficied
image file gets executed even if other image files were included later in the
configuration file.

Signed-off-by: Jorge Lucángeli Obes
---
diff --git a/qemu/vl.c b/qemu/vl.c
index fcc899b..88cefd2 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -42,8 +42,8 @@
 #include <netinet/in.h>
 #include <dirent.h>
 #include <netdb.h>
-#ifdef _BSD
 #include <sys/stat.h>
+#ifdef _BSD
 #ifndef __APPLE__
 #include <libutil.h>
 #endif
@@ -6367,9 +6367,16 @@ int main_loop(void)
 void help(void)
 {
     printf("QEMU PC emulator version " QEMU_VERSION ", Copyright (c)
2003-2007 Fabrice Bellard\n"
-           "usage: %s [options] [disk_image]\n"
+           "usage: %s [options] [disk_image|folder]\n"
            "\n"
+#ifdef _WIN32
            "'disk_image' is a raw hard image image for IDE hard disk 0\n"
+#else
+           "'disk_image' is a raw hard image image for IDE hard disk 0 or\n"
+           "'folder' is a folder with a file 'config' containing in
the first line\n"
+           "the name of an image file inside the folder and in the
rest of the file\n"
+           "options separated by ' ' or '\\n'\n"
+#endif
            "\n"
            "Standard options:\n"
            "-M machine      select emulated machine (-M ? for list)\n"
@@ -6892,6 +6899,20 @@ void qemu_get_launch_info(int *argc, char
***argv, int *opt_daemonize, const cha
     *opt_incoming = incoming;
 }

+char *dir_file_cat(const char *folder, const char *file) {
+    int foldlen = strlen(folder);
+    int filelen = strlen(file);
+    int reslen = foldlen + 1 + filelen + 1;
+
+    char *res = malloc(sizeof(char) * reslen);
+
+    pstrcpy(res, reslen, folder);
+    strncat(res, "/", 1);
+    strncat(res, file, filelen);
+
+    return res;
+}
+
 int main(int argc, char **argv)
 {
 #ifdef CONFIG_GDBSTUB
@@ -7003,7 +7024,120 @@ int main(int argc, char **argv)

     nb_nics = 0;
     /* default mac address of the first network interface */
+
+#ifndef _WIN32
+#define DIR_CMDLINE_SIZE 1<<13
+    int hd_found = 0;
+    char *dir, *opts;
+    struct stat *s = NULL;
+
+    optind = 1;
+    for(;;) {
+        if (optind >= argc)
+            break;
+
+        dir = argv[optind++];
+
+        if (dir[0] != '-') {
+            hd_found = 1;
+            break;
+        }
+    }

+    if (hd_found) {
+        s = malloc(sizeof(*s));
+
+        if (stat(dir, s) < 0) {
+            /* Error */
+            fprintf(stderr, "unable to stat: '%s'\n",
+                    dir);
+            exit(1);
+        }
+
+        if (S_ISDIR(s->st_mode)) {
+            /* The user specified a directory, search for ./config */
+            int configlen = strlen(dir);
+            configlen += 8; /* "/config\0" */
+            char *config = malloc(sizeof(char) * configlen);
+
+            pstrcpy(config, configlen, dir);
+            strncat(config, "/config", 7);
+
+            int fd_config;
+
+            if ((fd_config = open(config, 0)) < 0) {
+                /* Error */
+                if (errno == ENOENT)
+                    fprintf(stderr, "config file not found: '%s'\n",
+                            config);
+                else
+                    fprintf(stderr, "unable to open config file: '%s'\n",
+                            config);
+                exit(1);
+            }
+
+            opts = malloc(sizeof(char) * (DIR_CMDLINE_SIZE));
+
+            ssize_t readb = read(fd_config, opts, (DIR_CMDLINE_SIZE) - 1);
+
+            opts[readb] = '\0';
+
+            char *filename = strsep(&opts, "\n");
+
+            if (filename == NULL) {
+                /* Error */
+                fprintf(stderr, "malformed configuration file: '%s'\n",
+                        config);
+                exit(1);
+            } else if (strchr(filename, '/') != NULL) {
+                /* Error */
+                fprintf(stderr, "'%s' may point outside folder '%s'\n"
+                                "avoid using '/' in config file\n",
+                        filename, dir);
+                exit(1);
+            }
+
+            char tmpopts[DIR_CMDLINE_SIZE];
+            int done = 0;
+            unsigned int nbtoks = 0;
+            char *tok;
+
+            pstrcpy(tmpopts, DIR_CMDLINE_SIZE, opts);
+
+            do {
+                tok = strtok(nbtoks == 0? tmpopts : NULL, " \n");
+
+                if (tok != NULL)
+                    nbtoks++;
+                else
+                    done = 1;
+            } while (!done);
+
+            if (nbtoks > 0) {
+                char **argvprime = malloc((nbtoks + argc + 1) * sizeof(char*));
+
+                argvprime[0] = argv[0];
+
+                for (i = 0; i < nbtoks; i++)
+                    argvprime[i + 1] = strtok(i == 0? opts : NULL, " \n");
+
+                for (i = 1; i < argc; i++)
+                    argvprime[nbtoks + i] = argv[i];
+
+                argvprime[nbtoks + argc] = dir_file_cat(dir, filename);
+
+                argv = argvprime;
+                argc = nbtoks + argc + 1;
+
+                for (i = 0; i < argc; i++)
+                    printf("argv[%d] = %s\n", i, argv[i]);
+            }
+        }
+    }
+
+    free(s);
+#endif
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
@@ -7773,5 +7907,10 @@ int main(int argc, char **argv)

     main_loop();
     quit_timers();
+
+    /* argv was overwritten when parsing config file */
+    if (hd_found)
+    	free(argv);
+
     return 0;
 }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: exec_dir.patch --]
[-- Type: text/x-patch; name="exec_dir.patch", Size: 6111 bytes --]

This patch allows QEMU to execute a directory with a special format.       

This patch allows storing command line options in a configuration file inside  
a directory and then directly executing that directory. A simple check is
included to prevent the configuration file to access image files outside
the executed directory. Extra command line options can be passed on invocation,
which will take precedence over the ones stored in the configuration file.

The configuration file specifies, on its first line, the image file to use.
The rest of the file specifies command line options separated by spaces or
newlines. Careful reconstruction of the command line makes sure the speficied
image file gets executed even if other image files were included later in the
configuration file.

Signed-off-by: Jorge Lucángeli Obes
---
diff --git a/qemu/vl.c b/qemu/vl.c
index fcc899b..88cefd2 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -42,8 +42,8 @@
 #include <netinet/in.h>
 #include <dirent.h>
 #include <netdb.h>
-#ifdef _BSD
 #include <sys/stat.h>
+#ifdef _BSD
 #ifndef __APPLE__
 #include <libutil.h>
 #endif
@@ -6367,9 +6367,16 @@ int main_loop(void)
 void help(void)
 {
     printf("QEMU PC emulator version " QEMU_VERSION ", Copyright (c) 2003-2007 Fabrice Bellard\n"
-           "usage: %s [options] [disk_image]\n"
+           "usage: %s [options] [disk_image|folder]\n"
            "\n"
+#ifdef _WIN32
            "'disk_image' is a raw hard image image for IDE hard disk 0\n"
+#else
+           "'disk_image' is a raw hard image image for IDE hard disk 0 or\n"
+           "'folder' is a folder with a file 'config' containing in the first line\n"
+           "the name of an image file inside the folder and in the rest of the file\n"
+           "options separated by ' ' or '\\n'\n"
+#endif
            "\n"
            "Standard options:\n"
            "-M machine      select emulated machine (-M ? for list)\n"
@@ -6892,6 +6899,20 @@ void qemu_get_launch_info(int *argc, char ***argv, int *opt_daemonize, const cha
     *opt_incoming = incoming;
 }
 
+char *dir_file_cat(const char *folder, const char *file) {
+    int foldlen = strlen(folder);
+    int filelen = strlen(file);
+    int reslen = foldlen + 1 + filelen + 1;
+
+    char *res = malloc(sizeof(char) * reslen);
+
+    pstrcpy(res, reslen, folder);
+    strncat(res, "/", 1);
+    strncat(res, file, filelen);
+
+    return res;
+}
+
 int main(int argc, char **argv)
 {
 #ifdef CONFIG_GDBSTUB
@@ -7003,7 +7024,120 @@ int main(int argc, char **argv)
 
     nb_nics = 0;
     /* default mac address of the first network interface */
+
+#ifndef _WIN32
+#define DIR_CMDLINE_SIZE 1<<13
+    int hd_found = 0;
+    char *dir, *opts;
+    struct stat *s = NULL;
+
+    optind = 1;
+    for(;;) {
+        if (optind >= argc)
+            break;
+
+        dir = argv[optind++];
+
+        if (dir[0] != '-') {
+            hd_found = 1;
+            break;
+        }
+    }
     
+    if (hd_found) {
+        s = malloc(sizeof(*s));
+
+        if (stat(dir, s) < 0) {
+            /* Error */
+            fprintf(stderr, "unable to stat: '%s'\n",
+                    dir);
+            exit(1);
+        }
+
+        if (S_ISDIR(s->st_mode)) {
+            /* The user specified a directory, search for ./config */
+            int configlen = strlen(dir);
+            configlen += 8; /* "/config\0" */
+            char *config = malloc(sizeof(char) * configlen);
+
+            pstrcpy(config, configlen, dir);
+            strncat(config, "/config", 7);
+
+            int fd_config;
+
+            if ((fd_config = open(config, 0)) < 0) {
+                /* Error */
+                if (errno == ENOENT)
+                    fprintf(stderr, "config file not found: '%s'\n",
+                            config);
+                else
+                    fprintf(stderr, "unable to open config file: '%s'\n",
+                            config);
+                exit(1);
+            }
+
+            opts = malloc(sizeof(char) * (DIR_CMDLINE_SIZE));
+
+            ssize_t readb = read(fd_config, opts, (DIR_CMDLINE_SIZE) - 1);
+
+            opts[readb] = '\0';
+
+            char *filename = strsep(&opts, "\n");
+
+            if (filename == NULL) {
+                /* Error */
+                fprintf(stderr, "malformed configuration file: '%s'\n",
+                        config);
+                exit(1);
+            } else if (strchr(filename, '/') != NULL) {
+                /* Error */
+                fprintf(stderr, "'%s' may point outside folder '%s'\n"
+                                "avoid using '/' in config file\n",
+                        filename, dir);
+                exit(1);
+            }
+
+            char tmpopts[DIR_CMDLINE_SIZE];
+            int done = 0;
+            unsigned int nbtoks = 0;
+            char *tok;
+ 
+            pstrcpy(tmpopts, DIR_CMDLINE_SIZE, opts);
+
+            do {
+                tok = strtok(nbtoks == 0? tmpopts : NULL, " \n");
+
+                if (tok != NULL)
+                    nbtoks++;
+                else
+                    done = 1;
+            } while (!done);
+
+            if (nbtoks > 0) {
+                char **argvprime = malloc((nbtoks + argc + 1) * sizeof(char*));
+
+                argvprime[0] = argv[0];
+
+                for (i = 0; i < nbtoks; i++)
+                    argvprime[i + 1] = strtok(i == 0? opts : NULL, " \n");
+
+                for (i = 1; i < argc; i++)
+                    argvprime[nbtoks + i] = argv[i];
+
+                argvprime[nbtoks + argc] = dir_file_cat(dir, filename);
+
+                argv = argvprime;
+                argc = nbtoks + argc + 1;
+
+                for (i = 0; i < argc; i++)
+                    printf("argv[%d] = %s\n", i, argv[i]);
+            }
+        }
+    }
+
+    free(s);
+#endif
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
@@ -7773,5 +7907,10 @@ int main(int argc, char **argv)
 
     main_loop();
     quit_timers();
+
+    /* argv was overwritten when parsing config file */
+    if (hd_found)
+    	free(argv);
+
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Anthony Liguori]

It makes little sense to pass a directory when you can pass a config
file and assume that the directory the config file is in is the CWD.

For instance, if vm.cfg contained just the command line arguments, you
could have:

MyImage/vm.cfg:        -hda disk0.qcow -m 512
MyImage/disk0.qcow:    <disk image>

And then do:

qemu -c MyImage/vm.cfg

Regards,

Anthony Liguori


On Fri, 2007-08-31 at 15:19 -0300, Jorge Lucángeli Obes wrote:
> Hi all,
> 
> The last time this issue was discussed, the executable-directory idea
> gained more consensus than the snapshot-based idea. This patch
> implements my perception of the first idea. Non-Windows for now, as I
> can't test on a Windows system. Suggestions and constructive criticism
> more than welcome.
> 
> Cheers,
> Jorge
> 
> This patch allows QEMU to execute a directory with a special format.
> 
> This patch allows storing command line options in a configuration file inside
> a directory and then directly executing that directory. A simple check is
> included to prevent the configuration file to access image files outside
> the executed directory. Extra command line options can be passed on invocation,
> which will take precedence over the ones stored in the configuration file.
> 
> The configuration file specifies, on its first line, the image file to use.
> The rest of the file specifies command line options separated by spaces or
> newlines. Careful reconstruction of the command line makes sure the speficied
> image file gets executed even if other image files were included later in the
> configuration file.
> 
> Signed-off-by: Jorge Lucángeli Obes
> ---
> diff --git a/qemu/vl.c b/qemu/vl.c
> index fcc899b..88cefd2 100644
> --- a/qemu/vl.c
> +++ b/qemu/vl.c
> @@ -42,8 +42,8 @@
>  #include <netinet/in.h>
>  #include <dirent.h>
>  #include <netdb.h>
> -#ifdef _BSD
>  #include <sys/stat.h>
> +#ifdef _BSD
>  #ifndef __APPLE__
>  #include <libutil.h>
>  #endif
> @@ -6367,9 +6367,16 @@ int main_loop(void)
>  void help(void)
>  {
>      printf("QEMU PC emulator version " QEMU_VERSION ", Copyright (c)
> 2003-2007 Fabrice Bellard\n"
> -           "usage: %s [options] [disk_image]\n"
> +           "usage: %s [options] [disk_image|folder]\n"
>             "\n"
> +#ifdef _WIN32
>             "'disk_image' is a raw hard image image for IDE hard disk 0\n"
> +#else
> +           "'disk_image' is a raw hard image image for IDE hard disk 0 or\n"
> +           "'folder' is a folder with a file 'config' containing in
> the first line\n"
> +           "the name of an image file inside the folder and in the
> rest of the file\n"
> +           "options separated by ' ' or '\\n'\n"
> +#endif
>             "\n"
>             "Standard options:\n"
>             "-M machine      select emulated machine (-M ? for list)\n"
> @@ -6892,6 +6899,20 @@ void qemu_get_launch_info(int *argc, char
> ***argv, int *opt_daemonize, const cha
>      *opt_incoming = incoming;
>  }
> 
> +char *dir_file_cat(const char *folder, const char *file) {
> +    int foldlen = strlen(folder);
> +    int filelen = strlen(file);
> +    int reslen = foldlen + 1 + filelen + 1;
> +
> +    char *res = malloc(sizeof(char) * reslen);
> +
> +    pstrcpy(res, reslen, folder);
> +    strncat(res, "/", 1);
> +    strncat(res, file, filelen);
> +
> +    return res;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  #ifdef CONFIG_GDBSTUB
> @@ -7003,7 +7024,120 @@ int main(int argc, char **argv)
> 
>      nb_nics = 0;
>      /* default mac address of the first network interface */
> +
> +#ifndef _WIN32
> +#define DIR_CMDLINE_SIZE 1<<13
> +    int hd_found = 0;
> +    char *dir, *opts;
> +    struct stat *s = NULL;
> +
> +    optind = 1;
> +    for(;;) {
> +        if (optind >= argc)
> +            break;
> +
> +        dir = argv[optind++];
> +
> +        if (dir[0] != '-') {
> +            hd_found = 1;
> +            break;
> +        }
> +    }
> 
> +    if (hd_found) {
> +        s = malloc(sizeof(*s));
> +
> +        if (stat(dir, s) < 0) {
> +            /* Error */
> +            fprintf(stderr, "unable to stat: '%s'\n",
> +                    dir);
> +            exit(1);
> +        }
> +
> +        if (S_ISDIR(s->st_mode)) {
> +            /* The user specified a directory, search for ./config */
> +            int configlen = strlen(dir);
> +            configlen += 8; /* "/config\0" */
> +            char *config = malloc(sizeof(char) * configlen);
> +
> +            pstrcpy(config, configlen, dir);
> +            strncat(config, "/config", 7);
> +
> +            int fd_config;
> +
> +            if ((fd_config = open(config, 0)) < 0) {
> +                /* Error */
> +                if (errno == ENOENT)
> +                    fprintf(stderr, "config file not found: '%s'\n",
> +                            config);
> +                else
> +                    fprintf(stderr, "unable to open config file: '%s'\n",
> +                            config);
> +                exit(1);
> +            }
> +
> +            opts = malloc(sizeof(char) * (DIR_CMDLINE_SIZE));
> +
> +            ssize_t readb = read(fd_config, opts, (DIR_CMDLINE_SIZE) - 1);
> +
> +            opts[readb] = '\0';
> +
> +            char *filename = strsep(&opts, "\n");
> +
> +            if (filename == NULL) {
> +                /* Error */
> +                fprintf(stderr, "malformed configuration file: '%s'\n",
> +                        config);
> +                exit(1);
> +            } else if (strchr(filename, '/') != NULL) {
> +                /* Error */
> +                fprintf(stderr, "'%s' may point outside folder '%s'\n"
> +                                "avoid using '/' in config file\n",
> +                        filename, dir);
> +                exit(1);
> +            }
> +
> +            char tmpopts[DIR_CMDLINE_SIZE];
> +            int done = 0;
> +            unsigned int nbtoks = 0;
> +            char *tok;
> +
> +            pstrcpy(tmpopts, DIR_CMDLINE_SIZE, opts);
> +
> +            do {
> +                tok = strtok(nbtoks == 0? tmpopts : NULL, " \n");
> +
> +                if (tok != NULL)
> +                    nbtoks++;
> +                else
> +                    done = 1;
> +            } while (!done);
> +
> +            if (nbtoks > 0) {
> +                char **argvprime = malloc((nbtoks + argc + 1) * sizeof(char*));
> +
> +                argvprime[0] = argv[0];
> +
> +                for (i = 0; i < nbtoks; i++)
> +                    argvprime[i + 1] = strtok(i == 0? opts : NULL, " \n");
> +
> +                for (i = 1; i < argc; i++)
> +                    argvprime[nbtoks + i] = argv[i];
> +
> +                argvprime[nbtoks + argc] = dir_file_cat(dir, filename);
> +
> +                argv = argvprime;
> +                argc = nbtoks + argc + 1;
> +
> +                for (i = 0; i < argc; i++)
> +                    printf("argv[%d] = %s\n", i, argv[i]);
> +            }
> +        }
> +    }
> +
> +    free(s);
> +#endif
> +
>      optind = 1;
>      for(;;) {
>          if (optind >= argc)
> @@ -7773,5 +7907,10 @@ int main(int argc, char **argv)
> 
>      main_loop();
>      quit_timers();
> +
> +    /* argv was overwritten when parsing config file */
> +    if (hd_found)
> +    	free(argv);
> +
>      return 0;
>  }
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >>  http://get.splunk.com/
> _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Jorge Lucángeli Obes]

On 8/31/07, Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org> wrote:
> It makes little sense to pass a directory when you can pass a config
> file and assume that the directory the config file is in is the CWD.
>
> For instance, if vm.cfg contained just the command line arguments, you
> could have:
>
> MyImage/vm.cfg:        -hda disk0.qcow -m 512
> MyImage/disk0.qcow:    <disk image>
>
> And then do:
>
> qemu -c MyImage/vm.cfg

I thought about this when coding, and it's true. Maybe we could just
leave a new explicit config file option. I insist on making this as
simple as possible, that's why I  chose to use the config file as a
"written-down" command line.

If this were the case, we could remove the restriction of having the
config file refer only to images on the same directory, as the use of
the config file would be explicit. Thoughts?

Cheers,
Jorge

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Anthony Liguori]

On Fri, 2007-08-31 at 16:05 -0300, Jorge Lucángeli Obes wrote:
> On 8/31/07, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > It makes little sense to pass a directory when you can pass a config
> > file and assume that the directory the config file is in is the CWD.
> >
> > For instance, if vm.cfg contained just the command line arguments, you
> > could have:
> >
> > MyImage/vm.cfg:        -hda disk0.qcow -m 512
> > MyImage/disk0.qcow:    <disk image>
> >
> > And then do:
> >
> > qemu -c MyImage/vm.cfg
> 
> I thought about this when coding, and it's true. Maybe we could just
> leave a new explicit config file option. I insist on making this as
> simple as possible, that's why I  chose to use the config file as a
> "written-down" command line.
> 
> If this were the case, we could remove the restriction of having the
> config file refer only to images on the same directory, as the use of
> the config file would be explicit. Thoughts?

Yes.  It should be a very simple patch.

Regards,

Anthony Liguori

> Cheers,
> Jorge


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [Qemu-devel] Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Markus Hitter]

Am 31.08.2007 um 20:54 schrieb Anthony Liguori:

> It makes little sense to pass a directory when you can pass a config
> file and assume that the directory the config file is in is the CWD.

In fact, most people having designed bundle-type document formats  
came to a different conclusion: <http://en.wikipedia.org/wiki/Bundle_ 
(NEXTSTEP)>. Typically, bundles are opaque and appear like a single  
file to the desktop user.

> [...] And then do:
>
> qemu -c MyImage/vm.cfg

In opposite to "qemu -c MyImage" ?

Why do you want the user to do extra typing? There's one config in  
one directory, so typing the config file name is just redundant.


To me, Jorge's implementation looks just fine.


>> +           "usage: %s [options] [disk_image|folder]\n"
                 "usage: %s [options] [diskimage | bundle]\n"
                                                   ^^^^^^
Go ahead an call the baby by it's name?


m$0.02
Markus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/





-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [Qemu-devel] Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Andreas Färber]

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]


Am 01.09.2007 um 13:02 schrieb Markus Hitter:

>
> Am 31.08.2007 um 20:54 schrieb Anthony Liguori:
>
>> It makes little sense to pass a directory when you can pass a config
>> file and assume that the directory the config file is in is the CWD.
>
> In fact, most people having designed bundle-type document formats  
> came to a different conclusion: <http://en.wikipedia.org/wiki/ 
> Bundle_(NEXTSTEP)>. Typically, bundles are opaque and appear like a  
> single file to the desktop user.
>
>> [...] And then do:
>>
>> qemu -c MyImage/vm.cfg
>
> In opposite to "qemu -c MyImage" ?
>
> Why do you want the user to do extra typing? There's one config in  
> one directory, so typing the config file name is just redundant.
>
>
> To me, Jorge's implementation looks just fine.

I support the idea of having a bundle for the machine and naming only  
that bundle.

However the bundle still needs an extension. Like I already pointed  
out, on Mac OS X Q uses .qvm for its Qemu guest bundles, making it  
"qemu -c MyGuest.qvm". Or alternatively maybe .qemu?

And I don't understand why, when going along with the bundle idea,  
you are suddenly creating a new configuration file format of your  
own. There is a standard format, XML or text based, which can be  
easily parsed with OSX APIs (and thus likely *Step APIs as well) as a  
dictionary, allowing structured information to be stored and thus  
allowing Qemu backends to store their own settings alongside. See  
attached one my bundles' configuration.plist file (XML serialized) -  
relevant is only the Arguments entry, everything else is GUI  
specific. I'm not saying this format (the structure) were perfect but  
the underlying "property list" format (key, value-type) is standard  
for such bundles, so instead of re-inventing the wheel please take a  
look at how it's being done!

With Q already using this technique and QEMU possibly using bundles  
as well, it would seem like a good idea to synchronize the two  
efforts so that in the end we get only one bundle format instead of  
one for each frontend.

Thanks,

Andreas

[-- Attachment #2: configuration.plist --]
[-- Type: application/octet-stream, Size: 2546 bytes --]

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
	<key>About</key>
	<dict>
		<key>Author</key>
		<string>Q</string>
		<key>Copyright</key>
		<string>none</string>
		<key>Date</key>
		<date>2007-05-31T18:26:13Z</date>
		<key>Description</key>
		<string>Q guest PC</string>
	</dict>
	<key>Arguments</key>
	<string> -m 256 -net nic -net user -hda Harddisk_1.qcow2 -cdrom /Users/andreas/Q/debian-40r0-sparc-DVD-1.iso -boot c -redir tcp:2200::22 -redir tcp:1770::177 -redir udp:1770::177 -redir tcp:6000::6000 -redir tcp:6001::6001 -redir tcp:6002::6002 -redir tcp:6003::6003 -redir tcp:6004::6004 -redir tcp:6005::6005 -redir udp:6000::6000 -redir udp:6001::6001 -redir udp:6002::6002 -redir udp:6003::6003 -redir udp:6004::6004 -redir udp:6005::6005  -g 1024x768x8</string>
	<key>Network</key>
	<dict>
		<key>Redirect</key>
		<array>
			<dict>
				<key>Description</key>
				<string>This will let you access the guests SSH service from your host at port 2200.</string>
				<key>Enabled</key>
				<true/>
				<key>Name</key>
				<string>Remote Login (SSH)</string>
				<key>TCP-Ports-Guest</key>
				<string>22</string>
				<key>TCP-Ports-Host</key>
				<string>2200</string>
				<key>UDP-Ports-Guest</key>
				<string></string>
				<key>UDP-Ports-Host</key>
				<string></string>
			</dict>
			<dict>
				<key>Enabled</key>
				<true/>
				<key>Name</key>
				<string>XDMCP</string>
				<key>TCP-Ports-Guest</key>
				<string>177</string>
				<key>TCP-Ports-Host</key>
				<string>1770</string>
				<key>UDP-Ports-Guest</key>
				<string>177</string>
				<key>UDP-Ports-Host</key>
				<string>1770</string>
			</dict>
			<dict>
				<key>Enabled</key>
				<true/>
				<key>Name</key>
				<string>Test2</string>
				<key>TCP-Ports-Guest</key>
				<string>6000-6005</string>
				<key>TCP-Ports-Host</key>
				<string>6000-6005</string>
				<key>UDP-Ports-Guest</key>
				<string>6000-6005</string>
				<key>UDP-Ports-Host</key>
				<string>6000-6005</string>
			</dict>
		</array>
	</dict>
	<key>PC Data</key>
	<dict>
		<key>architecture</key>
		<string>SPARC</string>
		<key>name</key>
		<string>Debian Sparc</string>
		<key>state</key>
		<string>shutdown</string>
	</dict>
	<key>Temporary</key>
	<dict>
		<key>-cocoapath</key>
		<string>/Users/andreas/Documents/QEMU/Debian Sparc.qvm</string>
		<key>-hda</key>
		<string>Harddisk_1.qcow2</string>
	</dict>
	<key>Version</key>
	<string>0.2.0.Q</string>
</dict>
</plist>

[-- Attachment #3: Type: text/plain, Size: 1 bytes --]



[-- Attachment #4: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #5: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [Qemu-devel] Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Jorge Lucángeli Obes]

> And I don't understand why, when going along with the bundle idea,
> you are suddenly creating a new configuration file format of your
> own. There is a standard format, XML or text based, which can be
> easily parsed with OSX APIs (and thus likely *Step APIs as well) as a
> dictionary, allowing structured information to be stored and thus
> allowing Qemu backends to store their own settings alongside. See
> attached one my bundles' configuration.plist file (XML serialized) -
> relevant is only the Arguments entry, everything else is GUI
> specific. I'm not saying this format (the structure) were perfect but
> the underlying "property list" format (key, value-type) is standard
> for such bundles, so instead of re-inventing the wheel please take a
> look at how it's being done!

I would like to think that having command line options separated by
spaces or newlines is a new configuration file format, but it's not.
This patch is not meant as a replacement for libvirt or any other
backend. It's just a replacement for shell scripts that serve the only
purpose of storing command line options. It should be a simple
solution for a simple problem. The complex solution for the complex
problem is already there and is called libvirt. I don't want to have
QEMU parse serialized XML just to replace my '-m 512 -soundhw es1370
-net nic,model=rtl8139' command line. I would simply like to store
that command line somewhere.

Anyways, it's obvious that this is a delicate issue for many people
here. I think the most non-disruptive way of doing this is the '-c'
command line option. It doesn't change QEMU default behaviour, it
doesn't add new hyphen-less options. I insist on the fact that this
should be a simple solution for a simple problem.

Cheers,
Jorge

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [Qemu-devel] Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Andreas Färber]

Am 01.09.2007 um 20:45 schrieb Jorge Lucángeli Obes:

>> And I don't understand why, when going along with the bundle idea,
>> you are suddenly creating a new configuration file format of your
>> own. There is a standard format, XML or text based, which can be
>> easily parsed with OSX APIs (and thus likely *Step APIs as well) as a
>> dictionary, allowing structured information to be stored and thus
>> allowing Qemu backends to store their own settings alongside. See
>> attached one my bundles' configuration.plist file (XML serialized) -
>> relevant is only the Arguments entry, everything else is GUI
>> specific. I'm not saying this format (the structure) were perfect but
>> the underlying "property list" format (key, value-type) is standard
>> for such bundles, so instead of re-inventing the wheel please take a
>> look at how it's being done!
>
> I would like to think that having command line options separated by
> spaces or newlines is a new configuration file format, but it's not.

> This patch is not meant as a replacement for libvirt or any other
> backend. It's just a replacement for shell scripts that serve the only
> purpose of storing command line options. It should be a simple
> solution for a simple problem. The complex solution for the complex
> problem is already there and is called libvirt. I don't want to have
> QEMU parse serialized XML just to replace my '-m 512 -soundhw es1370
> -net nic,model=rtl8139' command line. I would simply like to store
> that command line somewhere.
>
> Anyways, it's obvious that this is a delicate issue for many people
> here. I think the most non-disruptive way of doing this is the '-c'
> command line option. It doesn't change QEMU default behaviour, it
> doesn't add new hyphen-less options. I insist on the fact that this
> should be a simple solution for a simple problem.

What you are basically doing is taking up the concept of a bundle but  
call it directory, do not give it a mandatory folder name extension  
and limit the usefulness of the configuration file to your personal  
needs.

The configuration file format you are proposing is new because you  
are proposing it now while, as one example, Q has previously  
introduced the concept of bundling a Qemu machine on the Mac. And  
the .plist format has existed for bundles even long before that.

Think about it: If you force frontends to use their own configuration  
files inside the bundle because you want to keep yours simple then  
you force frontends to parse two different configuration files.  
Whereas you yourself just said parsing one XML file was already too  
much for you! Standardizing a more advanced configuration file format  
here and now would enable frontends to exchange such bundles,  
retaining their information. By saying you just want a replacement  
for your command line scripts you are ignoring that other people and  
projects may have more advanced needs.

Oh and this has nothing to do with any virtualization libraries,  
virtualization is not what I (or Q) do so that is no solution at all.  
This is all about invoking QEMU.

So in the end it simply means that you are taking an existing concept  
and apply it half-heartedly and short-sighted: I might be wrong but  
it seems you have a *nix viewpoint, are not used to working with  
bundles and therefore re-inventing them differently. There are in  
fact "real" bundles on many Linux systems, have a look at pcsc-lite,  
e.g. /usr/lib/pcsc/drivers/ifd-ccid.bundle. This driver bundle has  
the default extension of .bundle, obviously not being opened as a  
document by a user, but users do start up virtual machines with QEMU  
so a custom extension is useful and necessary to detect that the  
directory represents in fact a bundle and more specifically a QEMU  
machine bundle.

Andreas
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [Qemu-devel] Re: [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)

[Jorge Lucángeli Obes]

On 9/1/07, Andreas Färber <andreas.faerber-S0/GAf8tV78@public.gmane.org> wrote:
>
> What you are basically doing is taking up the concept of a bundle but
> call it directory, do not give it a mandatory folder name extension
> and limit the usefulness of the configuration file to your personal
> needs.

I think the problem here is that the scope of this change is not
clear. I _really_ wish to keep this simple. I _really_ wish to avoid
having giant command lines and useless shell scripts. I don't mean to
reinvent the concept of a bundle. Using directories like that seemed a
good way of achieving the purpose of _not_ having to deal with command
lines. Now, it seems that a config file switch is a better choice. I
hardly think that being able to store command line options in a plain
text file is just a matter of _my_ personal needs.

> The configuration file format you are proposing is new because you
> are proposing it now while, as one example, Q has previously
> introduced the concept of bundling a Qemu machine on the Mac. And
> the .plist format has existed for bundles even long before that.
>
> Think about it: If you force frontends to use their own configuration
> files inside the bundle because you want to keep yours simple then
> you force frontends to parse two different configuration files.
> Whereas you yourself just said parsing one XML file was already too
> much for you! Standardizing a more advanced configuration file format
> here and now would enable frontends to exchange such bundles,
> retaining their information. By saying you just want a replacement
> for your command line scripts you are ignoring that other people and
> projects may have more advanced needs.

Those people that have more advanced needs can use the frontends. This
is not meant (necessarily) for frontends. It's meant exactly to
replace command line scripts.

> Oh and this has nothing to do with any virtualization libraries,
> virtualization is not what I (or Q) do so that is no solution at all.
> This is all about invoking QEMU.

libvirt can be perfectly used to invoke QEMU. The name might be
ambiguous, but the functionality is not. In fact, using or not using
KVM with QEMU reduces to selecting a checkbox in libvirt. It's just
another frontend.

> So in the end it simply means that you are taking an existing concept
> and apply it half-heartedly and short-sighted: I might be wrong but
> it seems you have a *nix viewpoint, are not used to working with
> bundles and therefore re-inventing them differently. There are in
> fact "real" bundles on many Linux systems, have a look at pcsc-lite,
> e.g. /usr/lib/pcsc/drivers/ifd-ccid.bundle. This driver bundle has
> the default extension of .bundle, obviously not being opened as a
> document by a user, but users do start up virtual machines with QEMU
> so a custom extension is useful and necessary to detect that the
> directory represents in fact a bundle and more specifically a QEMU
> machine bundle.

This is probably true. I never intended to implement the full concept
of a bundle. Again, I was looking for a way to avoid writing gigantic
command lines; a way that would have consensus in the QEMU community.
As I said in my last mail, the config file switch seems more suited to
do this. For what I intended to solve, implementing the whole "bundle"
thing is overkill. I'd rather not move the complexity of the "bundle"
into QEMU, but rather leave it on the frontends. So, let's just have a
simple way of storing command line options in a config file; a way
that does not conflict with existing frontends.

Cheers,
Jorge

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/