> -----Original Message----- > From: Klaus Jensen <[email protected]> > Sent: Monday, September 28, 2020 3:52 AM > To: Dmitry Fomichev <[email protected]> > Cc: Keith Busch <[email protected]>; Klaus Jensen > <[email protected]>; Kevin Wolf <[email protected]>; Philippe > Mathieu-Daudé <[email protected]>; Maxim Levitsky > <[email protected]>; Fam Zheng <[email protected]>; Niklas Cassel > <[email protected]>; Damien Le Moal <[email protected]>; > [email protected]; [email protected]; Alistair Francis > <[email protected]>; Matias Bjorling <[email protected]> > Subject: Re: [PATCH v5 13/14] hw/block/nvme: Use zone metadata file for > persistence > > On Sep 28 11:35, 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 <[email protected]> > > --- > > hw/block/nvme-ns.c | 341 > ++++++++++++++++++++++++++++++++++++++++-- > > hw/block/nvme-ns.h | 33 ++++ > > hw/block/nvme.c | 2 + > > hw/block/trace-events | 1 + > > 4 files changed, 362 insertions(+), 15 deletions(-) > > > > diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c > > index 47751f2d54..a94021da81 100644 > > --- a/hw/block/nvme-ns.c > > +++ b/hw/block/nvme-ns.c > > @@ -293,12 +421,180 @@ static void > nvme_init_zone_meta(NvmeNamespace *ns) > > i--; > > } > > } > > + > > + if (ns->params.zone_file) { > > + nvme_set_zone_meta_dirty(ns); > > + } > > +} > > + > > +static int nvme_open_zone_file(NvmeNamespace *ns, bool *init_meta, > > + Error **errp) > > +{ > > + Object *file_be; > > + HostMemoryBackend *fb; > > + struct stat statbuf; > > + int ret; > > + > > + ret = stat(ns->params.zone_file, &statbuf); > > + if (ret && errno == ENOENT) { > > + *init_meta = true; > > + } else if (!S_ISREG(statbuf.st_mode)) { > > + error_setg(errp, "\"%s\" is not a regular file", > > + ns->params.zone_file); > > + return -1; > > + } > > + > > + file_be = object_new(TYPE_MEMORY_BACKEND_FILE); > > + object_property_set_str(file_be, "mem-path", ns->params.zone_file, > > + &error_abort); > > + object_property_set_int(file_be, "size", ns->meta_size, &error_abort); > > + object_property_set_bool(file_be, "share", true, &error_abort); > > + object_property_set_bool(file_be, "discard-data", false, &error_abort); > > + if (!user_creatable_complete(USER_CREATABLE(file_be), errp)) { > > + object_unref(file_be); > > + return -1; > > + } > > + object_property_add_child(OBJECT(ns), "_fb", file_be); > > + object_unref(file_be); > > + > > + fb = MEMORY_BACKEND(file_be); > > + ns->zone_mr = host_memory_backend_get_memory(fb); > > + > > + return 0; > > +} > > + > > +static int nvme_map_zone_file(NvmeNamespace *ns, bool *init_meta) > > +{ > > + ns->zone_meta = (void *)memory_region_get_ram_ptr(ns->zone_mr); > > I forgot that the HostMemoryBackend doesn't magically make the memory > available to the device, so of course this is still needed. > > Anyway. > > No reason for me to keep complaining about this. I do not like it, I > will not ACK it and I think I made my reasons pretty clear.
So, memory_region_msync() is ok, but memory_region_get_ram_ptr() is not?? This is the same API! You are really splitting hairs here to suit your agenda. Moving goal posts again.... The "I do not like it" part is priceless. It is great that we have mail archives available.
