Re: [Qemu-devel] [RFC PATCH V1 2/2] arm_boot: conditionalised dtb command line update

[Peter Crosthwaite]

On Fri, Jun 1, 2012 at 11:56 AM, Peter Crosthwaite
<address@hidden> wrote:
> On Fri, Jun 1, 2012 at 11:44 AM, Peter Maydell <address@hidden> wrote:
>> On 1 June 2012 02:16, Peter A. G. Crosthwaite
>> <address@hidden> wrote:
>>> @@ -240,10 +241,13 @@ static int load_dtb(target_phys_addr_t addr, const 
>>> struct arm_boot_info *binfo)
>>>         fprintf(stderr, "couldn't set /memory/reg\n");
>>>     }
>>>
>>> -    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
>>> -                                      binfo->kernel_cmdline);
>>> -    if (rc < 0) {
>>> -        fprintf(stderr, "couldn't set /chosen/bootargs\n");
>>> +    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>>> +    if (machine_opts && qemu_opt_get(machine_opts, "append")) {
>>> +        rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
>>> +                                          binfo->kernel_cmdline);
>>> +        if (rc < 0) {
>>> +            fprintf(stderr, "couldn't set /chosen/bootargs\n");
>>> +        }
>>>     }
>>
>> Can you just check for binfo->kernel_cmdline being NULL or not
>> rather than rereading the option via qemu_opt_get? The latter
>> seems pretty ugly.
>>
>
> No, it wont work,
>
> vl.c will populate it with "\"\"":
>

Correction,

vl.c will populate it with an empty string. The problem still stands
though that an empty string explicitly passed to -append should mean
wipe out dtb command line.

>    if (machine_opts) {
> ...
>        kernel_cmdline = qemu_opt_get(machine_opts, "append");
>    } else {
>        kernel_filename = initrd_filename = kernel_cmdline = NULL;
>    }
>
>    if (!kernel_cmdline) {
>        kernel_cmdline = "";
>    }
>
>
> binfo->kernel_cmdline will not be null on the omission of -append.
>
> I did it this way as its the only way I can see where you can
> determine whether or not -apend happened. You could strcmp with "\"\""
> and use that as your condition, but then you have a possibly piece of
> policy that an empty command means no update (i.e. you should still be
> able to explictly -apend "\"\"" to wipe out the dtb command line).
>
> Regards,
>
> Peter
>
>> -- PMM