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

[Anthony Liguori <anthony@codemonkey.ws>]

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(?))
>