[Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Jorge Lucángeli Obes]

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

This patch makes QEMU check for command line options stored in qcow2 images.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
Signed-off-by: Jorge Lucángeli Obes <t4m5yn@gmail.com>
---
diff --git a/qemu/vl.c b/qemu/vl.c
index 4ad39f1..1d28794 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -184,6 +184,11 @@ int time_drift_fix = 0;
 const char *cpu_vendor_string;

 /***********************************************************/
+/* Image annotation support */
+
+char *img_get_annot(char *filename);
+
+/***********************************************************/
 /* x86 ISA bus support */

 target_phys_addr_t isa_mem_base = 0;
@@ -6917,6 +6922,13 @@ int main(int argc, char **argv)
     char usb_devices[MAX_USB_CMDLINE][128];
     int usb_devices_index;
     int fds[2];
+    char *tmpannot;
+    char annot[1024];
+    int done = 0;
+    unsigned int nbtoks = 0;
+    char *tok;
+    BlockDriver *drv;
+    BlockDriverState *bs;

     saved_argc = argc;
     saved_argv = argv;
@@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
     nb_nics = 0;
     /* default mac address of the first network interface */

+    bdrv_init();
+
+    drv = bdrv_find_format("qcow2");
+
+    if (argc > 1 && argv[1][0] != '-') {
+        bs = bdrv_new("");
+        if (!bs) {
+            fprintf(stderr, "Not enough memory");
+            exit(1);
+        }
+        if (bdrv_open2(bs, argv[1], 0, drv) < 0) {
+            fprintf(stderr, "Could not open '%s'", argv[1]);
+            bdrv_delete(bs);
+            exit(1);
+        }
+
+        tmpannot = bdrv_get_annot(bs, "commandline_args");
+        if (tmpannot) {
+            pstrcpy(annot, 1024, tmpannot);
+
+            do {
+                tok = strtok(nbtoks == 0? tmpannot : NULL, " ");
+
+                if (tok != NULL)
+                    nbtoks++;
+                else
+                    done = 1;
+            } while (!done);
+
+            free(tmpannot);
+
+            if (nbtoks > 0) {
+                char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
+
+                for (i = 0; i < argc; i++)
+                    argvprime[i] = argv[i];
+
+                for (i = 0; i < nbtoks; i++)
+                    argvprime[i + argc] = strtok(i == 0? annot : NULL, " ");
+
+                argv = argvprime;
+                argc = argc + nbtoks;
+
+                for (i = 0; i < nbtoks + 2; i++)
+                    printf("argv[%d] = %s\n", i, argv[i]);
+
+            }
+        }
+
+        bdrv_delete(bs);
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
@@ -7558,7 +7622,6 @@ int main(int argc, char **argv)
 #endif

     /* we always create the cdrom drive, even if no disk is there */
-    bdrv_init();
     if (cdrom_index >= 0) {
         bs_table[cdrom_index] = bdrv_new("cdrom");
         bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM);
@@ -7764,5 +7827,8 @@ int main(int argc, char **argv)

     main_loop();
     quit_timers();
+
+    /* was reassigned above so it needs free()ing */
+    free(argv);
     return 0;
 }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: qemu.patch --]
[-- Type: text/x-patch; name=qemu.patch; charset=ANSI_X3.4-1968, Size: 3096 bytes --]

This patch makes QEMU check for command line options stored in qcow2 images.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
Signed-off-by: Jorge Lucángeli Obes <t4m5yn@gmail.com>
---
diff --git a/qemu/vl.c b/qemu/vl.c
index 4ad39f1..1d28794 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -184,6 +184,11 @@ int time_drift_fix = 0;
 const char *cpu_vendor_string;
 
 /***********************************************************/
+/* Image annotation support */
+
+char *img_get_annot(char *filename);
+
+/***********************************************************/
 /* x86 ISA bus support */
 
 target_phys_addr_t isa_mem_base = 0;
@@ -6917,6 +6922,13 @@ int main(int argc, char **argv)
     char usb_devices[MAX_USB_CMDLINE][128];
     int usb_devices_index;
     int fds[2];
+    char *tmpannot;
+    char annot[1024];
+    int done = 0;
+    unsigned int nbtoks = 0;
+    char *tok;
+    BlockDriver *drv;
+    BlockDriverState *bs;
 
     saved_argc = argc;
     saved_argv = argv;
@@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
     nb_nics = 0;
     /* default mac address of the first network interface */
     
+    bdrv_init();
+
+    drv = bdrv_find_format("qcow2");
+    
+    if (argc > 1 && argv[1][0] != '-') {
+        bs = bdrv_new("");
+        if (!bs) {
+            fprintf(stderr, "Not enough memory");
+            exit(1);
+        }
+        if (bdrv_open2(bs, argv[1], 0, drv) < 0) {
+            fprintf(stderr, "Could not open '%s'", argv[1]);
+            bdrv_delete(bs);
+            exit(1);
+        }
+
+        tmpannot = bdrv_get_annot(bs, "commandline_args");
+        if (tmpannot) {
+            pstrcpy(annot, 1024, tmpannot);
+
+            do {
+                tok = strtok(nbtoks == 0? tmpannot : NULL, " ");
+
+                if (tok != NULL)
+                    nbtoks++;
+                else
+                    done = 1;
+            } while (!done);
+
+            free(tmpannot);            
+
+            if (nbtoks > 0) {
+                char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
+
+                for (i = 0; i < argc; i++)
+                    argvprime[i] = argv[i];
+
+                for (i = 0; i < nbtoks; i++)
+                    argvprime[i + argc] = strtok(i == 0? annot : NULL, " ");
+
+                argv = argvprime;
+                argc = argc + nbtoks;
+
+                for (i = 0; i < nbtoks + 2; i++)
+                    printf("argv[%d] = %s\n", i, argv[i]);
+
+            }
+        }    
+
+        bdrv_delete(bs);
+    }
+      
     optind = 1;
     for(;;) {
         if (optind >= argc)
@@ -7558,7 +7622,6 @@ int main(int argc, char **argv)
 #endif
 
     /* we always create the cdrom drive, even if no disk is there */
-    bdrv_init();
     if (cdrom_index >= 0) {
         bs_table[cdrom_index] = bdrv_new("cdrom");
         bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM);
@@ -7764,5 +7827,8 @@ int main(int argc, char **argv)
 
     main_loop();
     quit_timers();
+
+    /* was reassigned above so it needs free()ing */
+    free(argv);
     return 0;
 }

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Daniel P. Berrange]

On Wed, Aug 08, 2007 at 04:52:58PM -0300, Jorge Luc?ngeli Obes wrote:
> This patch makes QEMU check for command line options stored in qcow2 images.

I think it is a bad idea from a security POV to automatically extract & use 
command line args from  a disk image like this without the admin explicitly 
requesting this capability. 

eg If I grabbed a demo disk image from a vendors' or community website I would
certainly not trust whatever args may happen to be embedded in the disk image
and thus do not want QEMU to be automatically running using them.

I'd recommend having some command line flag to turn this capability on. For
example a '--args PATH-TO-DISK' flag,

  qemu --args $HOME/fedora.qcow

Would extract args from the disk image & us them.

While traditional

  qemu $HOME/fedora.qcow

would *not* extract args.

> diff --git a/qemu/vl.c b/qemu/vl.c
> index 4ad39f1..1d28794 100644
> --- a/qemu/vl.c
> +++ b/qemu/vl.c
> @@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
>      nb_nics = 0;
>      /* default mac address of the first network interface */
> 
> +    bdrv_init();
> +
> +    drv = bdrv_find_format("qcow2");
> +
> +    if (argc > 1 && argv[1][0] != '-') {
> +        bs = bdrv_new("");
> +        if (!bs) {
> +            fprintf(stderr, "Not enough memory");
> +            exit(1);
> +        }
> +        if (bdrv_open2(bs, argv[1], 0, drv) < 0) {
> +            fprintf(stderr, "Could not open '%s'", argv[1]);
> +            bdrv_delete(bs);
> +            exit(1);
> +        }
> +
> +        tmpannot = bdrv_get_annot(bs, "commandline_args");
> +        if (tmpannot) {
> +            pstrcpy(annot, 1024, tmpannot);
> +
> +            do {
> +                tok = strtok(nbtoks == 0? tmpannot : NULL, " ");
> +
> +                if (tok != NULL)
> +                    nbtoks++;
> +                else
> +                    done = 1;
> +            } while (!done);
> +
> +            free(tmpannot);
> +
> +            if (nbtoks > 0) {
> +                char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
> +
> +                for (i = 0; i < argc; i++)
> +                    argvprime[i] = argv[i];
> +
> +                for (i = 0; i < nbtoks; i++)
> +                    argvprime[i + argc] = strtok(i == 0? annot : NULL, " ");
> +
> +                argv = argvprime;
> +                argc = argc + nbtoks;
> +
> +                for (i = 0; i < nbtoks + 2; i++)
> +                    printf("argv[%d] = %s\n", i, argv[i]);
> +
> +            }
> +        }
> +
> +        bdrv_delete(bs);
> +    }
> +
>      optind = 1;
>      for(;;) {
>          if (optind >= argc)


Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Wed, Aug 08, 2007 at 04:52:58PM -0300, Jorge Luc?ngeli Obes wrote:
>   
>> This patch makes QEMU check for command line options stored in qcow2 images.
>>     
>
> I think it is a bad idea from a security POV to automatically extract & use 
> command line args from  a disk image like this without the admin explicitly 
> requesting this capability. 
>
> eg If I grabbed a demo disk image from a vendors' or community website I would
> certainly not trust whatever args may happen to be embedded in the disk image
> and thus do not want QEMU to be automatically running using them.
>
> I'd recommend having some command line flag to turn this capability on. For
> example a '--args PATH-TO-DISK' flag,
>
>   qemu --args $HOME/fedora.qcow
>   

That's pretty nasty.  How do you specify which disk this is then?  I do 
agree with you that allowing arbitrary command line arguments in an 
image file is probably a bad idea.  I think the general idea of being 
able to launch a single image is useful but I suspect this is not the 
right way to do it.

What are some people thinking would want to be stored in the file?  Most 
of the command line options are more host specific than guest specific I 
think.  Maybe we can store a pseudo-config instead that only contains a 
subset of parameters (and therefore, wouldn't pose a security risk)?

Regards,

Anthony Liguori

> Would extract args from the disk image & us them.
>
> While traditional
>
>   qemu $HOME/fedora.qcow
>
> would *not* extract args.
>
>   
>> diff --git a/qemu/vl.c b/qemu/vl.c
>> index 4ad39f1..1d28794 100644
>> --- a/qemu/vl.c
>> +++ b/qemu/vl.c
>> @@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
>>      nb_nics = 0;
>>      /* default mac address of the first network interface */
>>
>> +    bdrv_init();
>> +
>> +    drv = bdrv_find_format("qcow2");
>> +
>> +    if (argc > 1 && argv[1][0] != '-') {
>> +        bs = bdrv_new("");
>> +        if (!bs) {
>> +            fprintf(stderr, "Not enough memory");
>> +            exit(1);
>> +        }
>> +        if (bdrv_open2(bs, argv[1], 0, drv) < 0) {
>> +            fprintf(stderr, "Could not open '%s'", argv[1]);
>> +            bdrv_delete(bs);
>> +            exit(1);
>> +        }
>> +
>> +        tmpannot = bdrv_get_annot(bs, "commandline_args");
>> +        if (tmpannot) {
>> +            pstrcpy(annot, 1024, tmpannot);
>> +
>> +            do {
>> +                tok = strtok(nbtoks == 0? tmpannot : NULL, " ");
>> +
>> +                if (tok != NULL)
>> +                    nbtoks++;
>> +                else
>> +                    done = 1;
>> +            } while (!done);
>> +
>> +            free(tmpannot);
>> +
>> +            if (nbtoks > 0) {
>> +                char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
>> +
>> +                for (i = 0; i < argc; i++)
>> +                    argvprime[i] = argv[i];
>> +
>> +                for (i = 0; i < nbtoks; i++)
>> +                    argvprime[i + argc] = strtok(i == 0? annot : NULL, " ");
>> +
>> +                argv = argvprime;
>> +                argc = argc + nbtoks;
>> +
>> +                for (i = 0; i < nbtoks + 2; i++)
>> +                    printf("argv[%d] = %s\n", i, argv[i]);
>> +
>> +            }
>> +        }
>> +
>> +        bdrv_delete(bs);
>> +    }
>> +
>>      optind = 1;
>>      for(;;) {
>>          if (optind >= argc)
>>     
>
>
> Dan.
>   

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Daniel P. Berrange]

On Thu, Aug 09, 2007 at 09:54:17AM -0500, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >On Wed, Aug 08, 2007 at 04:52:58PM -0300, Jorge Luc?ngeli Obes wrote:
> >  
> >>This patch makes QEMU check for command line options stored in qcow2 
> >>images.
> >>    
> >
> >I think it is a bad idea from a security POV to automatically extract & 
> >use command line args from  a disk image like this without the admin 
> >explicitly requesting this capability. 
> >
> >eg If I grabbed a demo disk image from a vendors' or community website I 
> >would
> >certainly not trust whatever args may happen to be embedded in the disk 
> >image
> >and thus do not want QEMU to be automatically running using them.
> >
> >I'd recommend having some command line flag to turn this capability on. For
> >example a '--args PATH-TO-DISK' flag,
> >
> >  qemu --args $HOME/fedora.qcow
> >  
> 
> That's pretty nasty.  How do you specify which disk this is then?  I do 

Well that's the same problem you have with the current patch - it just assumes
you have a single disk & its going to be hda.

> agree with you that allowing arbitrary command line arguments in an 
> image file is probably a bad idea.  I think the general idea of being 
> able to launch a single image is useful but I suspect this is not the 
> right way to do it.
> 
> What are some people thinking would want to be stored in the file?  Most 
> of the command line options are more host specific than guest specific I 
> think.  Maybe we can store a pseudo-config instead that only contains a 
> subset of parameters (and therefore, wouldn't pose a security risk)?

Yeah that's a much more open ended & interesting problem to address. I
personally wouldn't store command line arguments in the image at all.
I think we'd be better off providing a generic metadata  description of
the capabilities / requirements of the guest OS/kernel inside the image.
So any virtualization platform could read the metadata, determine whether
the guest is supported & then boot it with appropriate commands. 

Once you have a good metadata description, there's many options for how you
distribute it - in the qcow image, or in tar.gz containing the image, the 
metdata & any other relevant bits. We've been trying to prototype a sample
metdata description &  a virt-image tool for launching an image based on
its metadata using libvirt.

   http://people.redhat.com/dlutter/virt-image/

The same metadata could be used to build a qemu command line directly for
the non-libvirt usage cases.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Avi Kivity]

Anthony Liguori wrote:
>>
>> I think it is a bad idea from a security POV to automatically extract 
>> & use command line args from  a disk image like this without the 
>> admin explicitly requesting this capability.
>> eg If I grabbed a demo disk image from a vendors' or community 
>> website I would
>> certainly not trust whatever args may happen to be embedded in the 
>> disk image
>> and thus do not want QEMU to be automatically running using them.
>>
>> I'd recommend having some command line flag to turn this capability 
>> on. For
>> example a '--args PATH-TO-DISK' flag,
>>
>>   qemu --args $HOME/fedora.qcow
>>   
>
> That's pretty nasty.  How do you specify which disk this is then?  I 
> do agree with you that allowing arbitrary command line arguments in an 
> image file is probably a bad idea.  I think the general idea of being 
> able to launch a single image is useful but I suspect this is not the 
> right way to do it.
>
> What are some people thinking would want to be stored in the file?  
> Most of the command line options are more host specific than guest 
> specific I think.  Maybe we can store a pseudo-config instead that 
> only contains a subset of parameters (and therefore, wouldn't pose a 
> security risk)? 

Memory size, -hdb and -cdrom, processor count, networking setup.  The 
sort of things people push into ad-hoc scripts.

I expect this to be the low-end solution; with high end management 
applications storing configuration options in a database.

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>>
>>> I think it is a bad idea from a security POV to automatically 
>>> extract & use command line args from  a disk image like this without 
>>> the admin explicitly requesting this capability.
>>> eg If I grabbed a demo disk image from a vendors' or community 
>>> website I would
>>> certainly not trust whatever args may happen to be embedded in the 
>>> disk image
>>> and thus do not want QEMU to be automatically running using them.
>>>
>>> I'd recommend having some command line flag to turn this capability 
>>> on. For
>>> example a '--args PATH-TO-DISK' flag,
>>>
>>>   qemu --args $HOME/fedora.qcow
>>>   
>>
>> That's pretty nasty.  How do you specify which disk this is then?  I 
>> do agree with you that allowing arbitrary command line arguments in 
>> an image file is probably a bad idea.  I think the general idea of 
>> being able to launch a single image is useful but I suspect this is 
>> not the right way to do it.
>>
>> What are some people thinking would want to be stored in the file?  
>> Most of the command line options are more host specific than guest 
>> specific I think.  Maybe we can store a pseudo-config instead that 
>> only contains a subset of parameters (and therefore, wouldn't pose a 
>> security risk)? 
>
> Memory size, -hdb and -cdrom, processor count, networking setup.  The 
> sort of things people push into ad-hoc scripts.
>
> I expect this to be the low-end solution; with high end management 
> applications storing configuration options in a database.

If you're looking for a low-end solution, another possibility would be 
having a "new" file format which consisted of:

#!/path/to/qemu [<args> ...] <nl>
<standard disk image>

And then make the appropriate changes to QEMU such that it can skip the 
first line in a disk image file.  This has a few nice side effects.  The 
disk image is directly executable and it makes it very clear to the user 
that they have to trust the disk image.  The other nice thing is that it 
would work with file formats other than qcow2.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Avi Kivity]

Anthony Liguori wrote:
>
> If you're looking for a low-end solution, another possibility would be 
> having a "new" file format which consisted of:
>
> #!/path/to/qemu [<args> ...] <nl>
> <standard disk image>
>
> And then make the appropriate changes to QEMU such that it can skip 
> the first line in a disk image file.  This has a few nice side 
> effects.  The disk image is directly executable and it makes it very 
> clear to the user that they have to trust the disk image.  The other 
> nice thing is that it would work with file formats other than qcow2.

Well, it would be nice to align the disk image to a sector boundary (or, 
better, a page boundary).  But yes, a very good idea.

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>
>> If you're looking for a low-end solution, another possibility would 
>> be having a "new" file format which consisted of:
>>
>> #!/path/to/qemu [<args> ...] <nl>
>> <standard disk image>
>>
>> And then make the appropriate changes to QEMU such that it can skip 
>> the first line in a disk image file.  This has a few nice side 
>> effects.  The disk image is directly executable and it makes it very 
>> clear to the user that they have to trust the disk image.  The other 
>> nice thing is that it would work with file formats other than qcow2.
>
> Well, it would be nice to align the disk image to a sector boundary 
> (or, better, a page boundary).  But yes, a very good idea.

That has the very nice side effect of allowing you to edit the command 
line with a text editor provided you don't cross the sector boundary.  
Filling with spaces would be pretty nice.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Avi Kivity]

Anthony Liguori wrote:
>
> That has the very nice side effect of allowing you to edit the command 
> line with a text editor provided you don't cross the sector boundary.  
> Filling with spaces would be pretty nice.
>

You must have a very capable editor, if you can edit multi-gigabyte 
files ;) -- these days most editors load everything into RAM.

(and don't forget to select overwrite mode -- actually the non-aligned 
case is more editor friendly)

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Brian Wheeler]

On Thu, 2007-08-09 at 15:32 -0500, Anthony Liguori wrote:
> Avi Kivity wrote:
> > Anthony Liguori wrote:
> >>
> >> If you're looking for a low-end solution, another possibility would 
> >> be having a "new" file format which consisted of:
> >>
> >> #!/path/to/qemu [<args> ...] <nl>
> >> <standard disk image>
> >>
> >> And then make the appropriate changes to QEMU such that it can skip 
> >> the first line in a disk image file.  This has a few nice side 
> >> effects.  The disk image is directly executable and it makes it very 
> >> clear to the user that they have to trust the disk image.  The other 
> >> nice thing is that it would work with file formats other than qcow2.
> >
> > Well, it would be nice to align the disk image to a sector boundary 
> > (or, better, a page boundary).  But yes, a very good idea.
> 
> That has the very nice side effect of allowing you to edit the command 
> line with a text editor provided you don't cross the sector boundary.  
> Filling with spaces would be pretty nice.
> 


Its a bad idea.  Either have a line terminated file, or a pure binary
file, but don't mix them.  Besides firing up an editor to modify it
being a royal pain, mystery breakages will occur when someone has their
editor set to "insert mode" (which is the default) vs "overwrite mode"
and doesn't notice those extra spaces moving beyond the 512-byte mark.

Brian

> Regards,
> 
> Anthony Liguori
> 
> 
> 

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Brian Wheeler wrote:
> On Thu, 2007-08-09 at 15:32 -0500, Anthony Liguori wrote:
>   
>> Avi Kivity wrote:
>>     
>>> Anthony Liguori wrote:
>>>       
>>>> If you're looking for a low-end solution, another possibility would 
>>>> be having a "new" file format which consisted of:
>>>>
>>>> #!/path/to/qemu [<args> ...] <nl>
>>>> <standard disk image>
>>>>
>>>> And then make the appropriate changes to QEMU such that it can skip 
>>>> the first line in a disk image file.  This has a few nice side 
>>>> effects.  The disk image is directly executable and it makes it very 
>>>> clear to the user that they have to trust the disk image.  The other 
>>>> nice thing is that it would work with file formats other than qcow2.
>>>>         
>>> Well, it would be nice to align the disk image to a sector boundary 
>>> (or, better, a page boundary).  But yes, a very good idea.
>>>       
>> That has the very nice side effect of allowing you to edit the command 
>> line with a text editor provided you don't cross the sector boundary.  
>> Filling with spaces would be pretty nice.
>>
>>     
>
>
> Its a bad idea.  Either have a line terminated file, or a pure binary
> file, but don't mix them.  Besides firing up an editor to modify it
> being a royal pain, mystery breakages will occur when someone has their
> editor set to "insert mode" (which is the default) vs "overwrite mode"
> and doesn't notice those extra spaces moving beyond the 512-byte mark.
>
> Brian
>   

Assuming that you have a tool that edits the command line (I admit, 
using emacs is a bad idea), then aligning to a sector boundary means 
that you don't have to munge the data every time you update.  Since most 
command lines will be < 512 bytes probably, this means that you can 
usually get away with not having to copy any data on disk which is a 
very good thing when you have multi-gigabyte files.

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>
>>
>>     
>
>
>
>
>   

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[dmc]

Brian Wheeler wrote:
> On Thu, 2007-08-09 at 15:32 -0500, Anthony Liguori wrote:
>> Avi Kivity wrote:
>>> Anthony Liguori wrote:
>>>> If you're looking for a low-end solution, another possibility would 
>>>> be having a "new" file format which consisted of:
>>>>
>>>> #!/path/to/qemu [<args> ...] <nl>
>>>> <standard disk image>
>>>>
>>>> And then make the appropriate changes to QEMU such that it can skip 
>>>> the first line in a disk image file.  This has a few nice side 
>>>> effects.  The disk image is directly executable and it makes it very 
>>>> clear to the user that they have to trust the disk image.  The other 
>>>> nice thing is that it would work with file formats other than qcow2.
>>> Well, it would be nice to align the disk image to a sector boundary 
>>> (or, better, a page boundary).  But yes, a very good idea.
>> That has the very nice side effect of allowing you to edit the command 
>> line with a text editor provided you don't cross the sector boundary.  
>> Filling with spaces would be pretty nice.
>>
> 
> 
> Its a bad idea.  Either have a line terminated file, or a pure binary
> file, but don't mix them.  Besides firing up an editor to modify it
> being a royal pain, mystery breakages will occur when someone has their
> editor set to "insert mode" (which is the default) vs "overwrite mode"
> and doesn't notice those extra spaces moving beyond the 512-byte mark.

how about xml appended to image file, and size of xml appended to that? 
  And a tool to easily extract/replace the xml.  Just my $0.02...

-dmc

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Carlos A. M. dos Santos]

On 8/10/07, dmc <dmc@filteredperception.org> wrote:
> Brian Wheeler wrote:
> > On Thu, 2007-08-09 at 15:32 -0500, Anthony Liguori wrote:
> >> Avi Kivity wrote:
> >>> Anthony Liguori wrote:
> >>>> If you're looking for a low-end solution, another possibility would
> >>>> be having a "new" file format which consisted of:
> >>>>
> >>>> #!/path/to/qemu [<args> ...] <nl>
> >>>> <standard disk image>
> >>>>
> >>>> And then make the appropriate changes to QEMU such that it can skip
> >>>> the first line in a disk image file.  This has a few nice side
> >>>> effects.  The disk image is directly executable and it makes it very
> >>>> clear to the user that they have to trust the disk image.  The other
> >>>> nice thing is that it would work with file formats other than qcow2.
> >>> Well, it would be nice to align the disk image to a sector boundary
> >>> (or, better, a page boundary).  But yes, a very good idea.
> >> That has the very nice side effect of allowing you to edit the command
> >> line with a text editor provided you don't cross the sector boundary.
> >> Filling with spaces would be pretty nice.
> >>
> >
> >
> > Its a bad idea.  Either have a line terminated file, or a pure binary
> > file, but don't mix them.  Besides firing up an editor to modify it
> > being a royal pain, mystery breakages will occur when someone has their
> > editor set to "insert mode" (which is the default) vs "overwrite mode"
> > and doesn't notice those extra spaces moving beyond the 512-byte mark.
>
> how about xml appended to image file, and size of xml appended to that?
>   And a tool to easily extract/replace the xml.  Just my $0.02...

Keep it *simple*, please!

-- 
Carlos A. M. dos Santos

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Brian Wheeler]

On Thu, 2007-08-09 at 23:16 +0300, Avi Kivity wrote:
> Anthony Liguori wrote:
> >>
> >> I think it is a bad idea from a security POV to automatically extract 
> >> & use command line args from  a disk image like this without the 
> >> admin explicitly requesting this capability.
> >> eg If I grabbed a demo disk image from a vendors' or community 
> >> website I would
> >> certainly not trust whatever args may happen to be embedded in the 
> >> disk image
> >> and thus do not want QEMU to be automatically running using them.
> >>
> >> I'd recommend having some command line flag to turn this capability 
> >> on. For
> >> example a '--args PATH-TO-DISK' flag,
> >>
> >>   qemu --args $HOME/fedora.qcow
> >>   
> >
> > That's pretty nasty.  How do you specify which disk this is then?  I 
> > do agree with you that allowing arbitrary command line arguments in an 
> > image file is probably a bad idea.  I think the general idea of being 
> > able to launch a single image is useful but I suspect this is not the 
> > right way to do it.
> >
> > What are some people thinking would want to be stored in the file?  
> > Most of the command line options are more host specific than guest 
> > specific I think.  Maybe we can store a pseudo-config instead that 
> > only contains a subset of parameters (and therefore, wouldn't pose a 
> > security risk)? 
> 
> Memory size, -hdb and -cdrom, processor count, networking setup.  The 
> sort of things people push into ad-hoc scripts.
> 
> I expect this to be the low-end solution; with high end management 
> applications storing configuration options in a database.
> 
> 

Why not just save the options to a file and have qemu parse it?  That
way all of the security issues are taken care of, and it can be cross
platform (no need for a shell script and/or batch file) so it'd be
portable.

If the format was one flag per line (as if the command line got broken
up in pairs) as in "-hdb myfile.img" being on one line and "-fda
boot.img" on another line, then its easy to edit programically as well.

All of my shell scripts to start qemu tend to look like this:

qemu -hda disk0.img -net nic -net user -m 512 -localtime   $*

so I can pass one-time parameters as necessary (that's the $* at the
end) by specifying args when I invoke the script.  If qemu had a default
configuration file it looked for, and then you could specify one-or-more
configuration files to read in addition (later values overriding earlier
ones), then it seems like it'd work out for most if not all situations.



Brian

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Brian Wheeler wrote:
> On Thu, 2007-08-09 at 23:16 +0300, Avi Kivity wrote:
>   
>> Anthony Liguori wrote:
>>     
>>>> I think it is a bad idea from a security POV to automatically extract 
>>>> & use command line args from  a disk image like this without the 
>>>> admin explicitly requesting this capability.
>>>> eg If I grabbed a demo disk image from a vendors' or community 
>>>> website I would
>>>> certainly not trust whatever args may happen to be embedded in the 
>>>> disk image
>>>> and thus do not want QEMU to be automatically running using them.
>>>>
>>>> I'd recommend having some command line flag to turn this capability 
>>>> on. For
>>>> example a '--args PATH-TO-DISK' flag,
>>>>
>>>>   qemu --args $HOME/fedora.qcow
>>>>   
>>>>         
>>> That's pretty nasty.  How do you specify which disk this is then?  I 
>>> do agree with you that allowing arbitrary command line arguments in an 
>>> image file is probably a bad idea.  I think the general idea of being 
>>> able to launch a single image is useful but I suspect this is not the 
>>> right way to do it.
>>>
>>> What are some people thinking would want to be stored in the file?  
>>> Most of the command line options are more host specific than guest 
>>> specific I think.  Maybe we can store a pseudo-config instead that 
>>> only contains a subset of parameters (and therefore, wouldn't pose a 
>>> security risk)? 
>>>       
>> Memory size, -hdb and -cdrom, processor count, networking setup.  The 
>> sort of things people push into ad-hoc scripts.
>>
>> I expect this to be the low-end solution; with high end management 
>> applications storing configuration options in a database.
>>
>>
>>     
>
> Why not just save the options to a file and have qemu parse it?  That
> way all of the security issues are taken care of, and it can be cross
> platform (no need for a shell script and/or batch file) so it'd be
> portable.
>
> If the format was one flag per line (as if the command line got broken
> up in pairs) as in "-hdb myfile.img" being on one line and "-fda
> boot.img" on another line, then its easy to edit programically as well.
>
> All of my shell scripts to start qemu tend to look like this:
>
> qemu -hda disk0.img -net nic -net user -m 512 -localtime   $*
>
> so I can pass one-time parameters as necessary (that's the $* at the
> end) by specifying args when I invoke the script.  If qemu had a default
> configuration file it looked for, and then you could specify one-or-more
> configuration files to read in addition (later values overriding earlier
> ones), then it seems like it'd work out for most if not all situations.
>   

Yes, I agree config files are the proper solution here but that's a 
pretty big effort.  It may be valuable to have an intermediary solution.

Regards,

Anthony Liguori

>
> Brian
>
>
>
>
>
>
>   

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[andrzej zaborowski]

On 08/08/07, Jorge Lucángeli Obes <t4m5yn@gmail.com> wrote:
> This patch makes QEMU check for command line options stored in qcow2 images.
>
> Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
> Signed-off-by: Jorge Lucángeli Obes <t4m5yn@gmail.com>
> ---
> diff --git a/qemu/vl.c b/qemu/vl.c
> index 4ad39f1..1d28794 100644
> --- a/qemu/vl.c
> +++ b/qemu/vl.c
> @@ -184,6 +184,11 @@ int time_drift_fix = 0;
>  const char *cpu_vendor_string;
>
>  /***********************************************************/
> +/* Image annotation support */
> +
> +char *img_get_annot(char *filename);
> +
> +/***********************************************************/
>  /* x86 ISA bus support */
>
>  target_phys_addr_t isa_mem_base = 0;
> @@ -6917,6 +6922,13 @@ int main(int argc, char **argv)
>      char usb_devices[MAX_USB_CMDLINE][128];
>      int usb_devices_index;
>      int fds[2];
> +    char *tmpannot;
> +    char annot[1024];
> +    int done = 0;
> +    unsigned int nbtoks = 0;
> +    char *tok;
> +    BlockDriver *drv;
> +    BlockDriverState *bs;
>
>      saved_argc = argc;
>      saved_argv = argv;
> @@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
>      nb_nics = 0;
>      /* default mac address of the first network interface */
>
> +    bdrv_init();
> +
> +    drv = bdrv_find_format("qcow2");
> +
> +    if (argc > 1 && argv[1][0] != '-') {
> +        bs = bdrv_new("");
> +        if (!bs) {
> +            fprintf(stderr, "Not enough memory");
> +            exit(1);
> +        }
> +        if (bdrv_open2(bs, argv[1], 0, drv) < 0) {
> +            fprintf(stderr, "Could not open '%s'", argv[1]);
> +            bdrv_delete(bs);
> +            exit(1);
> +        }
> +
> +        tmpannot = bdrv_get_annot(bs, "commandline_args");
> +        if (tmpannot) {
> +            pstrcpy(annot, 1024, tmpannot);

I thought one of the arguments for storing config options was the
commandline length limit, which is 32k or so characters on most
systems.

> +
> +            do {
> +                tok = strtok(nbtoks == 0? tmpannot : NULL, " ");
> +
> +                if (tok != NULL)
> +                    nbtoks++;
> +                else
> +                    done = 1;
> +            } while (!done);
> +
> +            free(tmpannot);
> +
> +            if (nbtoks > 0) {
> +                char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
> +
> +                for (i = 0; i < argc; i++)
> +                    argvprime[i] = argv[i];
> +
> +                for (i = 0; i < nbtoks; i++)
> +                    argvprime[i + argc] = strtok(i == 0? annot : NULL, " ");
> +
> +                argv = argvprime;
> +                argc = argc + nbtoks;
> +
> +                for (i = 0; i < nbtoks + 2; i++)
> +                    printf("argv[%d] = %s\n", i, argv[i]);
> +
> +            }
> +        }
> +
> +        bdrv_delete(bs);
> +    }
> +
>      optind = 1;
>      for(;;) {
>          if (optind >= argc)
> @@ -7558,7 +7622,6 @@ int main(int argc, char **argv)
>  #endif
>
>      /* we always create the cdrom drive, even if no disk is there */
> -    bdrv_init();
>      if (cdrom_index >= 0) {
>          bs_table[cdrom_index] = bdrv_new("cdrom");
>          bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM);
> @@ -7764,5 +7827,8 @@ int main(int argc, char **argv)
>
>      main_loop();
>      quit_timers();
> +
> +    /* was reassigned above so it needs free()ing */
> +    free(argv);
>      return 0;
>  }
>
>

Yes, the file format starting with "#! /path/to/qemu" is a much better
idea, but the commandline length limitation has to be taken care of
separately, perhaps the switches should follow on next lines in the
file while the #! line shouldn't specify the switches.

Regards,
Andrzej (away until mid-September(?))

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Philip Boulain]

Yikes. I like the intent, but the idea of a previously just-data file format 
suddenly being able to imply "-hdb fat:rw:/home/" does not strike me as a good 
one. :/

andrzej zaborowski wrote:
> Yes, the file format starting with "#! /path/to/qemu" is a much better
> idea...

That should probably be "#!/usr/bin/env qemu", or something similar, if the 
intent is that "self-executing" image files are mostly zero-effort portable 
across (UNIX-y) host environments.

Anthony Liguori wrote:
> The disk image is directly executable and it makes it very clear to the user that they have to trust the disk image.

Only if qemu only read the embedded arguments in the case where it was executed 
as a script interpreter for the image, and/or only if the image's execute bit 
is set. In other words, this should prevent embedded arguments from being used:

   $ chmod -x dubious-image.qcow2
   $ qemu -hda dubious-image.qcow2

This also doesn't apply outside of UNIX-like environments, e.g. Windows; if 
someone had told Explorer to launch image files as "qemu.exe -hda (image)" 
(which is as close to shebanging a data file as you can really get), this could 
really be a nasty surprise.

LionsPhil

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Christian Brunschen]

On 11 Aug 2007, at 19:06, Philip Boulain wrote:

> Yikes. I like the intent, but the idea of a previously just-data  
> file format suddenly being able to imply "-hdb fat:rw:/home/" does  
> not strike me as a good one. :/

Hi all,

I'm nobody in particular in the world of qemu, and have certainly  
never contributed to it, but I hope you won't mind if I mention some  
thoughts.

It seems to me that overloading a simple data format to suddenly also  
include metadata is indeed a decision fraught with potential for  
unhappiness.

Much better would be to perhaps devise a container format that would  
embed a qemu configuration as well as a number of disk image files.  
Of course, to have such ini a single file is not necessarily very  
easy. However, one need only look at NeXTSTEP/OPENSTEP/Mac OS X to  
see a solution: use a directory.

Basically, one way to bundle a machine configuration together yet  
leave existing file formats unchanged is to define a standard  
directory structure that can contain a qemu configuration file (a set  
of command line options) as well as a number of related files (mainly  
disk images, but could also me BIOS ROM or similar of course). It  
doesn't need to be a complex structure: simply specify that a 'qemu  
machine directory' contains a file named 'config.qemu' in a specific  
format (with file references relative to the directory containing  
config.qemu). qemu, if given as its sole command line option the name  
of a directory, checks whether the directory contains a suitable  
config.qemu, and reads & process the file as if it were command line  
options (with the current directory.

Such a directory can be kept entirely platform independent, such that  
the entire directory could be moved or copied from one host to  
another. It would mean perhaps tar:ing or zip:ing it up, but  
certainly on Mac OS X such directories have proven to be a successful  
way of bundling things together (certainly helped by the widespread  
use of disk image files, through which distributing directories  
becomes entirely a non-problem).

Something like that _should_ offer most of the features that people  
want - a simple way to encapsulate both configuration and data,  
platform-independent, short and sweet to specify on the command line,  
without making any changes to existing file formats (particularly  
disk image ones).

Just a thought,

// Christian Brunschen

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Christian Brunschen wrote:
>
> On 11 Aug 2007, at 19:06, Philip Boulain wrote:
>
>> Yikes. I like the intent, but the idea of a previously just-data file 
>> format suddenly being able to imply "-hdb fat:rw:/home/" does not 
>> strike me as a good one. :/
>
> Hi all,
>
> I'm nobody in particular in the world of qemu, and have certainly 
> never contributed to it, but I hope you won't mind if I mention some 
> thoughts.
>
> It seems to me that overloading a simple data format to suddenly also 
> include metadata is indeed a decision fraught with potential for 
> unhappiness.
>
> Much better would be to perhaps devise a container format that would 
> embed a qemu configuration as well as a number of disk image files. Of 
> course, to have such ini a single file is not necessarily very easy. 
> However, one need only look at NeXTSTEP/OPENSTEP/Mac OS X to see a 
> solution: use a directory.

I expect we'll see something like this emerge in the future.

Regards,

ANthonY Liguori

> Basically, one way to bundle a machine configuration together yet 
> leave existing file formats unchanged is to define a standard 
> directory structure that can contain a qemu configuration file (a set 
> of command line options) as well as a number of related files (mainly 
> disk images, but could also me BIOS ROM or similar of course). It 
> doesn't need to be a complex structure: simply specify that a 'qemu 
> machine directory' contains a file named 'config.qemu' in a specific 
> format (with file references relative to the directory containing 
> config.qemu). qemu, if given as its sole command line option the name 
> of a directory, checks whether the directory contains a suitable 
> config.qemu, and reads & process the file as if it were command line 
> options (with the current directory.
>
> Such a directory can be kept entirely platform independent, such that 
> the entire directory could be moved or copied from one host to 
> another. It would mean perhaps tar:ing or zip:ing it up, but certainly 
> on Mac OS X such directories have proven to be a successful way of 
> bundling things together (certainly helped by the widespread use of 
> disk image files, through which distributing directories becomes 
> entirely a non-problem).
>
> Something like that _should_ offer most of the features that people 
> want - a simple way to encapsulate both configuration and data, 
> platform-independent, short and sweet to specify on the command line, 
> without making any changes to existing file formats (particularly disk 
> image ones).
>
> Just a thought,
>
> // Christian Brunschen
>
>
>
>

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

Philip Boulain wrote:
> Yikes. I like the intent, but the idea of a previously just-data file 
> format suddenly being able to imply "-hdb fat:rw:/home/" does not 
> strike me as a good one. :/

This is why directly executable is important so that the user realizes 
they must trust the image.

> andrzej zaborowski wrote:
>> Yes, the file format starting with "#! /path/to/qemu" is a much better
>> idea...
>
> That should probably be "#!/usr/bin/env qemu", or something similar, 
> if the intent is that "self-executing" image files are mostly 
> zero-effort portable across (UNIX-y) host environments.

I think the magic should just be "#!".  Whatever you put as the QEMU 
executable is your choice.  Separating the args to the next line 
actually does make it pretty portable.  See my previous post as to how 
it would work under Windows.

> Anthony Liguori wrote:
>> The disk image is directly executable and it makes it very clear to 
>> the user that they have to trust the disk image.
>
> Only if qemu only read the embedded arguments in the case where it was 
> executed as a script interpreter for the image, and/or only if the 
> image's execute bit is set. In other words, this should prevent 
> embedded arguments from being used:
>
>   $ chmod -x dubious-image.qcow2
>   $ qemu -hda dubious-image.qcow2

Yes, I think that another argument should be required as Dan suggested 
although I'd like something more explicit like "-read-args-from-image".  
In the case where the image was directly executable, it would be 
embedded as part of the interpreter arguments.

> This also doesn't apply outside of UNIX-like environments, e.g. 
> Windows; if someone had told Explorer to launch image files as 
> "qemu.exe -hda (image)" (which is as close to shebanging a data file 
> as you can really get), this could really be a nasty surprise.

I think this is covered by requiring the additional argument.

Regards,

Anthony Liguori

> LionsPhil
>
>
>
>

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Philip Boulain]

Anthony Liguori wrote:
> Philip Boulain wrote:
>> That should probably be "#!/usr/bin/env qemu", or something similar...
> I think the magic should just be "#!".  Whatever you put as the QEMU 
> executable is your choice.  Separating the args to the next line 
> actually does make it pretty portable.

Not really; you'd still have to use the editing tool (qemu-img?) to prep it for 
your particular QEMU binary location on that particular host. But, yes: being 
able to set arbitrary binaries is essential. Using env is probably a good 
/default/, though.

> Yes, I think that another argument should be required as Dan suggested 
> although I'd like something more explicit like "-read-args-from-image".  
> In the case where the image was directly executable, it would be 
> embedded as part of the interpreter arguments.

This works, so long as qemu disregards "-read-args-from-image" unless it's 
being called as an interpreter. Otherwise, you've put the lock and the key in 
the same place. :)

If an image says:
   #!/whatever/qemu -read-args-from-image
   -curious-and-interesting-flags

This should use those flags:
   $ ./image.qcow2

And this shouldn't, because you don't need to have made it executable:
   $ qemu -hda image.qcow2

(But this would:)
   $ qemu -read-args-from-image -hda image.qcow2

(Side thought: presumably, we're assuming that the in-file and on-command-line 
arguments are unioned, ideally with the latter taking precidence if mutually 
exclusive.)

>> This also doesn't apply outside of UNIX-like environments, e.g. 
>> Windows...
> I think this is covered by requiring the additional argument.

Agreed.

LionsPhil

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

On Sat, 2007-08-11 at 22:28 +0100, Philip Boulain wrote:
> > Yes, I think that another argument should be required as Dan suggested 
> > although I'd like something more explicit like "-read-args-from-image".  
> > In the case where the image was directly executable, it would be 
> > embedded as part of the interpreter arguments.
> 
> This works, so long as qemu disregards "-read-args-from-image" unless it's 
> being called as an interpreter. Otherwise, you've put the lock and the key in 
> the same place. :)
> 
> If an image says:
>    #!/whatever/qemu -read-args-from-image
>    -curious-and-interesting-flags
> 
> This should use those flags:
>    $ ./image.qcow2
> 
> And this shouldn't, because you don't need to have made it executable:
>    $ qemu -hda image.qcow2
> 
> (But this would:)
>    $ qemu -read-args-from-image -hda image.qcow2

Yes.  This is exactly what I think the behavior ought to be.

> (Side thought: presumably, we're assuming that the in-file and on-command-line 
> arguments are unioned, ideally with the latter taking precidence if mutually 
> exclusive.)

Yup, I was thinking the same thing too.  Once the KVM wiki comes back
online I'll write something up on a wiki page.

Regards,

Anthony Liguori

> >> This also doesn't apply outside of UNIX-like environments, e.g. 
> >> Windows...
> > I think this is covered by requiring the additional argument.
> 
> Agreed.
> 
> LionsPhil
> 
> 

Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Jorge Lucángeli Obes]

On 8/11/07, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On Sat, 2007-08-11 at 22:28 +0100, Philip Boulain wrote:
> > This works, so long as qemu disregards "-read-args-from-image" unless it's
> > being called as an interpreter. Otherwise, you've put the lock and the key in
> > the same place. :)
>
> Yes.  This is exactly what I think the behavior ought to be.

Ack.

> > (Side thought: presumably, we're assuming that the in-file and on-command-line
> > arguments are unioned, ideally with the latter taking precidence if mutually
> > exclusive.)
>
> Yup, I was thinking the same thing too.  Once the KVM wiki comes back
> online I'll write something up on a wiki page.

Great, that was also my idea. Anyone interested in taking a stab at
this one? I will try and start getting it done this week.

Cheers,
Jorge

Re: [kvm-devel] [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

On Mon, 2007-08-13 at 02:34 -0300, Jorge Lucángeli Obes wrote:
> On 8/11/07, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > On Sat, 2007-08-11 at 22:28 +0100, Philip Boulain wrote:
> > > This works, so long as qemu disregards "-read-args-from-image" unless it's
> > > being called as an interpreter. Otherwise, you've put the lock and the key in
> > > the same place. :)
> >
> > Yes.  This is exactly what I think the behavior ought to be.
> 
> Ack.
> 
> > > (Side thought: presumably, we're assuming that the in-file and on-command-line
> > > arguments are unioned, ideally with the latter taking precidence if mutually
> > > exclusive.)
> >
> > Yup, I was thinking the same thing too.  Once the KVM wiki comes back
> > online I'll write something up on a wiki page.
> 
> Great, that was also my idea. Anyone interested in taking a stab at
> this one? I will try and start getting it done this week.

http://kvm.qumranet.com/kvmwiki/Specs/StoringCommandLineInImage

Although it could use some more editing.

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] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images

[Anthony Liguori]

andrzej zaborowski wrote:
> On 08/08/07, Jorge Lucángeli Obes <t4m5yn@gmail.com> wrote:
>   
>> This patch makes QEMU check for command line options stored in qcow2 images.
>>
>> Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
>> Signed-off-by: Jorge Lucángeli Obes <t4m5yn@gmail.com>
>> ---
>> diff --git a/qemu/vl.c b/qemu/vl.c
>> index 4ad39f1..1d28794 100644
>> --- a/qemu/vl.c
>> +++ b/qemu/vl.c
>> @@ -184,6 +184,11 @@ int time_drift_fix = 0;
>>  const char *cpu_vendor_string;
>>
>>  /***********************************************************/
>> +/* Image annotation support */
>> +
>> +char *img_get_annot(char *filename);
>> +
>> +/***********************************************************/
>>  /* x86 ISA bus support */
>>
>>  target_phys_addr_t isa_mem_base = 0;
>> @@ -6917,6 +6922,13 @@ int main(int argc, char **argv)
>>      char usb_devices[MAX_USB_CMDLINE][128];
>>      int usb_devices_index;
>>      int fds[2];
>> +    char *tmpannot;
>> +    char annot[1024];
>> +    int done = 0;
>> +    unsigned int nbtoks = 0;
>> +    char *tok;
>> +    BlockDriver *drv;
>> +    BlockDriverState *bs;
>>
>>      saved_argc = argc;
>>      saved_argv = argv;
>> @@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
>>      nb_nics = 0;
>>      /* default mac address of the first network interface */
>>
>> +    bdrv_init();
>> +
>> +    drv = bdrv_find_format("qcow2");
>> +
>> +    if (argc > 1 && argv[1][0] != '-') {
>> +        bs = bdrv_new("");
>> +        if (!bs) {
>> +            fprintf(stderr, "Not enough memory");
>> +            exit(1);
>> +        }
>> +        if (bdrv_open2(bs, argv[1], 0, drv) < 0) {
>> +            fprintf(stderr, "Could not open '%s'", argv[1]);
>> +            bdrv_delete(bs);
>> +            exit(1);
>> +        }
>> +
>> +        tmpannot = bdrv_get_annot(bs, "commandline_args");
>> +        if (tmpannot) {
>> +            pstrcpy(annot, 1024, tmpannot);
>>     
>
> I thought one of the arguments for storing config options was the
> commandline length limit, which is 32k or so characters on most
> systems.
>
>   
>> +
>> +            do {
>> +                tok = strtok(nbtoks == 0? tmpannot : NULL, " ");
>> +
>> +                if (tok != NULL)
>> +                    nbtoks++;
>> +                else
>> +                    done = 1;
>> +            } while (!done);
>> +
>> +            free(tmpannot);
>> +
>> +            if (nbtoks > 0) {
>> +                char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
>> +
>> +                for (i = 0; i < argc; i++)
>> +                    argvprime[i] = argv[i];
>> +
>> +                for (i = 0; i < nbtoks; i++)
>> +                    argvprime[i + argc] = strtok(i == 0? annot : NULL, " ");
>> +
>> +                argv = argvprime;
>> +                argc = argc + nbtoks;
>> +
>> +                for (i = 0; i < nbtoks + 2; i++)
>> +                    printf("argv[%d] = %s\n", i, argv[i]);
>> +
>> +            }
>> +        }
>> +
>> +        bdrv_delete(bs);
>> +    }
>> +
>>      optind = 1;
>>      for(;;) {
>>          if (optind >= argc)
>> @@ -7558,7 +7622,6 @@ int main(int argc, char **argv)
>>  #endif
>>
>>      /* we always create the cdrom drive, even if no disk is there */
>> -    bdrv_init();
>>      if (cdrom_index >= 0) {
>>          bs_table[cdrom_index] = bdrv_new("cdrom");
>>          bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM);
>> @@ -7764,5 +7827,8 @@ int main(int argc, char **argv)
>>
>>      main_loop();
>>      quit_timers();
>> +
>> +    /* was reassigned above so it needs free()ing */
>> +    free(argv);
>>      return 0;
>>  }
>>
>>
>>     
>
> Yes, the file format starting with "#! /path/to/qemu" is a much better
> idea, but the commandline length limitation has to be taken care of
> separately, perhaps the switches should follow on next lines in the
> file while the #! line shouldn't specify the switches.
>   

Yes, I think that's a very good idea.  I think it should be extended 
slightly to incorporate Dan's original suggestion of using an option to 
specify that the command line ought to be read from the image.  So you'd 
end up with:

#!/path/to/qemu -read-args-from-image <nl>
-m 512 -net tap -net nic,model=rtl8139 <padding><nl>

Such that this whole section was sector aligned.  The "#!" is the magic 
for the disk image type.  On windows, you could completely omit 
/path/to/qemu and just set up an extension association that would 
associate .qemu with "/path/to/qemu -read-args-from-image %s" and then 
it would work equally well with Windows.

Regards,

Anthony Liguori

> Regards,
> Andrzej (away until mid-September(?))
>