Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver
On 2017-11-08 18:14, Kevin Wolf wrote: > Am 02.11.2017 um 17:18 hat Max Reitz geschrieben: >> On 2017-11-02 17:11, Alberto Garcia wrote: >>> On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote: QLIST_ENTRY(BlockDriver) list; + +/* Pointer to a NULL-terminated array of names of significant options that + * can be specified for bdrv_open(). A significant option is one that + * changes the data of a BDS. + * If this pointer is NULL, the array is considered empty. + * "filename" and "driver" are always considered significant. */ +const char *const *sgfnt_runtime_opts; }; >>> >>> Is sgfnt a common acronym? I actually had to look it up... >> >> I'm open for other suggestions to shorten "significant". :-) >> >> (Actually, I can't remember where I got it from. I think I just made it >> up, but then I don't know why I didn't use sgfcnt -- maybe because cnt >> looks either like count (or like something else, but that's a different >> matter)...?) > > Because in reality you were abbreviating "signifant"? ;-) I actually thought the very same, yes. ;-) > Anyway, I found the inline initialisation as part of BlockDriver rather > ugly. The places where you used a separate static const array look > much nicer. Maybe use that consistently in all places? Sure. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver
Am 02.11.2017 um 17:18 hat Max Reitz geschrieben: > On 2017-11-02 17:11, Alberto Garcia wrote: > > On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote: > >> QLIST_ENTRY(BlockDriver) list; > >> + > >> +/* Pointer to a NULL-terminated array of names of significant options > >> that > >> + * can be specified for bdrv_open(). A significant option is one that > >> + * changes the data of a BDS. > >> + * If this pointer is NULL, the array is considered empty. > >> + * "filename" and "driver" are always considered significant. */ > >> +const char *const *sgfnt_runtime_opts; > >> }; > > > > Is sgfnt a common acronym? I actually had to look it up... > > I'm open for other suggestions to shorten "significant". :-) > > (Actually, I can't remember where I got it from. I think I just made it > up, but then I don't know why I didn't use sgfcnt -- maybe because cnt > looks either like count (or like something else, but that's a different > matter)...?) Because in reality you were abbreviating "signifant"? ;-) Anyway, I found the inline initialisation as part of BlockDriver rather ugly. The places where you used a separate static const array look much nicer. Maybe use that consistently in all places? Kevin signature.asc Description: PGP signature
Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver
On Thu 02 Nov 2017 05:18:31 PM CET, Max Reitz wrote: > On 2017-11-02 17:11, Alberto Garcia wrote: >> On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote: >>> QLIST_ENTRY(BlockDriver) list; >>> + >>> +/* Pointer to a NULL-terminated array of names of significant options >>> that >>> + * can be specified for bdrv_open(). A significant option is one that >>> + * changes the data of a BDS. >>> + * If this pointer is NULL, the array is considered empty. >>> + * "filename" and "driver" are always considered significant. */ >>> +const char *const *sgfnt_runtime_opts; >>> }; >> >> Is sgfnt a common acronym? I actually had to look it up... > > I'm open for other suggestions to shorten "significant". :-) Nah, it's ok :) Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver
On 2017-11-02 17:11, Alberto Garcia wrote:
> On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote:
>> QLIST_ENTRY(BlockDriver) list;
>> +
>> +/* Pointer to a NULL-terminated array of names of significant options
>> that
>> + * can be specified for bdrv_open(). A significant option is one that
>> + * changes the data of a BDS.
>> + * If this pointer is NULL, the array is considered empty.
>> + * "filename" and "driver" are always considered significant. */
>> +const char *const *sgfnt_runtime_opts;
>> };
>
> Is sgfnt a common acronym? I actually had to look it up...
I'm open for other suggestions to shorten "significant". :-)
(Actually, I can't remember where I got it from. I think I just made it
up, but then I don't know why I didn't use sgfcnt -- maybe because cnt
looks either like count (or like something else, but that's a different
matter)...?)
Max
>> +static const char *const iscsi_sgfnt_runtime_opts[] = {
>> +"transport",
>> +"portal",
>> +"target",
>> +"user",
>> +"password",
>> +"password-secret",
>> +"lun",
>> +"initiator-name",
>> +"header-digest",
>> +
>> +NULL
>> +};
>
> Ideally it would be nice to have constants for all these options, but
> this is a pre-existing problem.
>
> Reviewed-by: Alberto Garcia
>
> Berto
>
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver
On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote:
> QLIST_ENTRY(BlockDriver) list;
> +
> +/* Pointer to a NULL-terminated array of names of significant options
> that
> + * can be specified for bdrv_open(). A significant option is one that
> + * changes the data of a BDS.
> + * If this pointer is NULL, the array is considered empty.
> + * "filename" and "driver" are always considered significant. */
> +const char *const *sgfnt_runtime_opts;
> };
Is sgfnt a common acronym? I actually had to look it up...
> +static const char *const iscsi_sgfnt_runtime_opts[] = {
> +"transport",
> +"portal",
> +"target",
> +"user",
> +"password",
> +"password-secret",
> +"lun",
> +"initiator-name",
> +"header-digest",
> +
> +NULL
> +};
Ideally it would be nice to have constants for all these options, but
this is a pre-existing problem.
Reviewed-by: Alberto Garcia
Berto
