Re: [PATCH v3 14/15] hw/block/nvme: Use zone metadata file for persistence

2020-09-15 Thread Klaus Jensen
On Sep 15 20:44, Dmitry Fomichev wrote:
> > -Original Message-
> > From: Klaus Jensen 
> > Sent: Tuesday, September 15, 2020 3:10 PM
> > To: Dmitry Fomichev 
> > Cc: Keith Busch ; Klaus Jensen
> > ; Kevin Wolf ; Philippe
> > Mathieu-Daudé ; Maxim Levitsky
> > ; Fam Zheng ; Niklas Cassel
> > ; Damien Le Moal ;
> > qemu-bl...@nongnu.org; qemu-devel@nongnu.org; Alistair Francis
> > ; Matias Bjorling 
> > Subject: Re: [PATCH v3 14/15] hw/block/nvme: Use zone metadata file for
> > persistence
> > 
> > On Sep 14 07:14, Dmitry Fomichev wrote:
> > > A ZNS drive that is emulated by this module is currently initialized
> > > with all zones Empty upon startup. However, actual ZNS SSDs save the
> > > state and condition of all zones in their internal NVRAM in the event
> > > of power loss. When such a drive is powered up again, it closes or
> > > finishes all zones that were open at the moment of shutdown. Besides
> > > that, the write pointer position as well as the state and condition
> > > of all zones is preserved across power-downs.
> > >
> > > This commit adds the capability to have a persistent zone metadata
> > > to the device. The new optional module property, "zone_file",
> > > is introduced. If added to the command line, this property specifies
> > > the name of the file that stores the zone metadata. If "zone_file" is
> > > omitted, the device will be initialized with all zones empty, the same
> > > as before.
> > >
> > > If zone metadata is configured to be persistent, then zone descriptor
> > > extensions also persist across controller shutdowns.
> > >
> > > Signed-off-by: Dmitry Fomichev 
> > 
> > This doesn't build on mingw.
> 
> Thanks for letting me know. I'll try to look into this. Do you cross-compile
> with mingw64?

Yup,

make docker-test-mingw@fedora



signature.asc
Description: PGP signature


RE: [PATCH v3 14/15] hw/block/nvme: Use zone metadata file for persistence

2020-09-15 Thread Dmitry Fomichev
> -Original Message-
> From: Klaus Jensen 
> Sent: Tuesday, September 15, 2020 3:10 PM
> To: Dmitry Fomichev 
> Cc: Keith Busch ; Klaus Jensen
> ; Kevin Wolf ; Philippe
> Mathieu-Daudé ; Maxim Levitsky
> ; Fam Zheng ; Niklas Cassel
> ; Damien Le Moal ;
> qemu-bl...@nongnu.org; qemu-devel@nongnu.org; Alistair Francis
> ; Matias Bjorling 
> Subject: Re: [PATCH v3 14/15] hw/block/nvme: Use zone metadata file for
> persistence
> 
> On Sep 14 07:14, Dmitry Fomichev wrote:
> > A ZNS drive that is emulated by this module is currently initialized
> > with all zones Empty upon startup. However, actual ZNS SSDs save the
> > state and condition of all zones in their internal NVRAM in the event
> > of power loss. When such a drive is powered up again, it closes or
> > finishes all zones that were open at the moment of shutdown. Besides
> > that, the write pointer position as well as the state and condition
> > of all zones is preserved across power-downs.
> >
> > This commit adds the capability to have a persistent zone metadata
> > to the device. The new optional module property, "zone_file",
> > is introduced. If added to the command line, this property specifies
> > the name of the file that stores the zone metadata. If "zone_file" is
> > omitted, the device will be initialized with all zones empty, the same
> > as before.
> >
> > If zone metadata is configured to be persistent, then zone descriptor
> > extensions also persist across controller shutdowns.
> >
> > Signed-off-by: Dmitry Fomichev 
> 
> This doesn't build on mingw.

Thanks for letting me know. I'll try to look into this. Do you cross-compile
with mingw64?

> 
> > ---
> >  hw/block/nvme.c | 370
> +---
> >  hw/block/nvme.h |  37 +
> >  2 files changed, 386 insertions(+), 21 deletions(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index b49ae83dd5..41f4c0dacd 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -3429,7 +3557,188 @@ static int nvme_init_zone_meta(NvmeCtrl *n,
> NvmeNamespace *ns,
> >  return 0;
> >  }
> >
> > -static void nvme_zoned_init_ctrl(NvmeCtrl *n, Error **errp)
> > +static int nvme_open_zone_file(NvmeCtrl *n, bool *init_meta)
> > +{
> > +struct stat statbuf;
> > +size_t fsize;
> > +int ret;
> > +
> > +ret = stat(n->params.zone_file, );
> > +if (ret && errno == ENOENT) {
> > +*init_meta = true;
> > +} else if (!S_ISREG(statbuf.st_mode)) {
> > +fprintf(stderr, "%s is not a regular file\n", strerror(errno));
> > +return -1;
> > +}
> > +
> > +n->zone_file_fd = open(n->params.zone_file,
> > +   O_RDWR | O_LARGEFILE | O_BINARY | O_CREAT, 644);
> 
> mode is wrong - I think you meant for it to be octal.

Nice catch, needs to be 0644...


Re: [PATCH v3 14/15] hw/block/nvme: Use zone metadata file for persistence

2020-09-15 Thread Klaus Jensen
On Sep 14 07:14, Dmitry Fomichev wrote:
> A ZNS drive that is emulated by this module is currently initialized
> with all zones Empty upon startup. However, actual ZNS SSDs save the
> state and condition of all zones in their internal NVRAM in the event
> of power loss. When such a drive is powered up again, it closes or
> finishes all zones that were open at the moment of shutdown. Besides
> that, the write pointer position as well as the state and condition
> of all zones is preserved across power-downs.
> 
> This commit adds the capability to have a persistent zone metadata
> to the device. The new optional module property, "zone_file",
> is introduced. If added to the command line, this property specifies
> the name of the file that stores the zone metadata. If "zone_file" is
> omitted, the device will be initialized with all zones empty, the same
> as before.
> 
> If zone metadata is configured to be persistent, then zone descriptor
> extensions also persist across controller shutdowns.
> 
> Signed-off-by: Dmitry Fomichev 

This doesn't build on mingw.

> ---
>  hw/block/nvme.c | 370 +---
>  hw/block/nvme.h |  37 +
>  2 files changed, 386 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index b49ae83dd5..41f4c0dacd 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -3429,7 +3557,188 @@ static int nvme_init_zone_meta(NvmeCtrl *n, 
> NvmeNamespace *ns,
>  return 0;
>  }
>  
> -static void nvme_zoned_init_ctrl(NvmeCtrl *n, Error **errp)
> +static int nvme_open_zone_file(NvmeCtrl *n, bool *init_meta)
> +{
> +struct stat statbuf;
> +size_t fsize;
> +int ret;
> +
> +ret = stat(n->params.zone_file, );
> +if (ret && errno == ENOENT) {
> +*init_meta = true;
> +} else if (!S_ISREG(statbuf.st_mode)) {
> +fprintf(stderr, "%s is not a regular file\n", strerror(errno));
> +return -1;
> +}
> +
> +n->zone_file_fd = open(n->params.zone_file,
> +   O_RDWR | O_LARGEFILE | O_BINARY | O_CREAT, 644);

mode is wrong - I think you meant for it to be octal.


signature.asc
Description: PGP signature