Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 11/13] nvdimm: allow setting the label-size to 0

[Haozhong Zhang]

On 06/15/18 16:04, David Hildenbrand wrote:
> It is inititally 0, so setting it to 0 should be allowed, too.

I'm fine with this change and believe nothing is broken in practice,
but what is expected by the user who sets a zero label size?

Look at nvdimm_dsm_device() which enables label DSMs only if the label
size is not smaller than 128 KB. If a user sets a zero label size
explicitly, does he/she expect those label DSMs are available in
guest?  (according to Intel spec, the minimal label size is 128
KBytes)

I think if it's allowed to set a zero label-size, it would be better
to document its difference from other non-zero values in docs/nvdimm.txt.

Thanks,
Haozhong

> 
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>  hw/mem/nvdimm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index db7d8c3050..df7646488b 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -52,9 +52,9 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, 
> const char *name,
>      if (local_err) {
>          goto out;
>      }
> -    if (value < MIN_NAMESPACE_LABEL_SIZE) {
> +    if (value && value < MIN_NAMESPACE_LABEL_SIZE) {
>          error_setg(&local_err, "Property '%s.%s' (0x%" PRIx64 ") is required"
> -                   " at least 0x%lx", object_get_typename(obj),
> +                   " either 0 or at least 0x%lx", object_get_typename(obj),
>                     name, value, MIN_NAMESPACE_LABEL_SIZE);
>          goto out;
>      }
> -- 
> 2.17.1
> 
>

signature.asc
Description: PGP signature