| Message ID | 20210114234612.795621-1-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] machine: add missing doc for memory-backend option | expand |
On 1/15/21 12:46 AM, Igor Mammedov wrote: > Add documentation for '-machine memory-backend' CLI option and > how to use it. > > And document that x-use-canonical-path-for-ramblock-id, > is considered to be stable to make sure it won't go away by accident. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > - add doc that x-use-canonical-path-for-ramblock-id is considered stable, > (Peter Krempa <pkrempa@redhat.com>) > --- > backends/hostmem.c | 10 ++++++++++ > qemu-options.hx | 28 +++++++++++++++++++++++++++- > 2 files changed, 37 insertions(+), 1 deletion(-) > > @@ -96,6 +97,31 @@ SRST > ``hmat=on|off`` > Enables or disables ACPI Heterogeneous Memory Attribute Table > (HMAT) support. The default is off. > + > + ``memory-backend='id'`` > + An alternative to legacy ``-mem-path`` and ``mem-prealloc`` options. > + Allows to use a memory backend as main RAM. > + > + For example: > + :: > + -object memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on > + -machine memory-backend=pc.ram > + -m 512M > + > + Migration compatibility note: > + a) as backend id one shall use value of 'default-ram-id', advertised by > + machine type (available via ``query-machines`` QMP command) > + b) for machine types 4.0 and older, user shall > + use ``x-use-canonical-path-for-ramblock-id=on`` backend option > + (this option must be considered stable, as if it didn't have the 'x-' > + prefix including deprecation period, as long as 4.0 and older machine > + types exists), > + if migration to/from old QEMU (<5.0) is expected. > + For example: > + :: > + -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=on > + -machine memory-backend=pc.ram > + -m 512M Igor, this doesn't correspond with your comment in bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1836043#c31 In fact, we had to turn the attribute OFF so that canonical path is not used. Isn't ON the default state anyway? Michal
On Thu, 14 Jan 2021 at 23:48, Igor Mammedov <imammedo@redhat.com> wrote: > > Add documentation for '-machine memory-backend' CLI option and > how to use it. > > And document that x-use-canonical-path-for-ramblock-id, > is considered to be stable to make sure it won't go away by accident. That's not what the x- prefix is supposed to mean. If we have an internal constraint that we mustn't delete the option in order to support some other must-be-stable interface (eg migration of some machines) we can document that in a comment, but that doesn't mean that we should document to users that direct use of an x-prefix option is supported as a stable interface. Alternatively, if the option is really stable for direct use by users then we should commit to making it so by removing the x-. thanks -- PMM
On Fri, Jan 15, 2021 at 10:02:04 +0000, Peter Maydell wrote: > On Thu, 14 Jan 2021 at 23:48, Igor Mammedov <imammedo@redhat.com> wrote: > > > > Add documentation for '-machine memory-backend' CLI option and > > how to use it. > > > > And document that x-use-canonical-path-for-ramblock-id, > > is considered to be stable to make sure it won't go away by accident. > > That's not what the x- prefix is supposed to mean. This is the exact reason I was asking on behalf of the libvirt team for adding such statement if we were to use it. We want guarantee that it's considered stable since without that it will not be accepted into libvirt. > If we have an internal constraint that we mustn't delete > the option in order to support some other must-be-stable > interface (eg migration of some machines) we can document > that in a comment, but that doesn't mean that we should > document to users that direct use of an x-prefix option > is supported as a stable interface. AFAIK the issue is that with the new approach to configure system memory (via a memory-backend=id, since the old one was deprecated) migration fails from/to older qemus ... > Alternatively, if the option is really stable for direct > use by users then we should commit to making it so by > removing the x-. ... thus the idea behind keeping this interface as is is it also fixes the migration compatibility for qemu 5.0/5.1/5.2 which were already released. Removing the 'x-' will fix it only starting with qemu-6.0 and any downstream which backports the removal of the prefix. Obviously not using 'x-' prefixed options is strongly preferred in libvirt.
On Fri, Jan 15, 2021 at 11:43:10 +0100, Peter Krempa wrote: > On Fri, Jan 15, 2021 at 10:02:04 +0000, Peter Maydell wrote: > > On Thu, 14 Jan 2021 at 23:48, Igor Mammedov <imammedo@redhat.com> wrote: [...] > Removing the 'x-' will fix it only starting with qemu-6.0 and any > downstream which backports the removal of the prefix. > > Obviously not using 'x-' prefixed options is strongly preferred in > libvirt. For reference, here are the relevant threads on libvir-list: v1: https://www.redhat.com/archives/libvir-list/2021-January/msg00601.html v2: https://www.redhat.com/archives/libvir-list/2021-January/msg00684.html And my objection to use 'x-' prefixed features without being guaranteed that it's considered stable (note that the premise is that it's supposed to fix already-released qemu versions thus not completely refusing using the existing naming) https://www.redhat.com/archives/libvir-list/2021-January/msg00606.html https://www.redhat.com/archives/libvir-list/2021-January/msg00699.html
On Fri, 15 Jan 2021 10:36:05 +0100 Michal Privoznik <mprivozn@redhat.com> wrote: > On 1/15/21 12:46 AM, Igor Mammedov wrote: > > Add documentation for '-machine memory-backend' CLI option and > > how to use it. > > > > And document that x-use-canonical-path-for-ramblock-id, > > is considered to be stable to make sure it won't go away by accident. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v2: > > - add doc that x-use-canonical-path-for-ramblock-id is considered stable, > > (Peter Krempa <pkrempa@redhat.com>) > > --- > > backends/hostmem.c | 10 ++++++++++ > > qemu-options.hx | 28 +++++++++++++++++++++++++++- > > 2 files changed, 37 insertions(+), 1 deletion(-) > > > > > > @@ -96,6 +97,31 @@ SRST > > ``hmat=on|off`` > > Enables or disables ACPI Heterogeneous Memory Attribute Table > > (HMAT) support. The default is off. > > + > > + ``memory-backend='id'`` > > + An alternative to legacy ``-mem-path`` and ``mem-prealloc`` options. > > + Allows to use a memory backend as main RAM. > > + > > + For example: > > + :: > > + -object memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on > > + -machine memory-backend=pc.ram > > + -m 512M > > + > > + Migration compatibility note: > > + a) as backend id one shall use value of 'default-ram-id', advertised by > > + machine type (available via ``query-machines`` QMP command) > > + b) for machine types 4.0 and older, user shall > > + use ``x-use-canonical-path-for-ramblock-id=on`` backend option > > + (this option must be considered stable, as if it didn't have the 'x-' > > + prefix including deprecation period, as long as 4.0 and older machine > > + types exists), > > + if migration to/from old QEMU (<5.0) is expected. > > + For example: > > + :: > > + -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=on > > + -machine memory-backend=pc.ram > > + -m 512M > > Igor, this doesn't correspond with your comment in bugzilla: > > https://bugzilla.redhat.com/show_bug.cgi?id=1836043#c31 > > In fact, we had to turn the attribute OFF so that canonical path is not > used. Isn't ON the default state anyway? indeed, it should be 'off', > > Michal > >
On Fri, 15 Jan 2021 10:02:04 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > On Thu, 14 Jan 2021 at 23:48, Igor Mammedov <imammedo@redhat.com> wrote: > > > > Add documentation for '-machine memory-backend' CLI option and > > how to use it. > > > > And document that x-use-canonical-path-for-ramblock-id, > > is considered to be stable to make sure it won't go away by accident. > > That's not what the x- prefix is supposed to mean. > If we have an internal constraint that we mustn't delete > the option in order to support some other must-be-stable > interface (eg migration of some machines) we can document > that in a comment, that was in v1, and Peter asked for adding assurance to help/doc as well. > but that doesn't mean that we should > document to users that direct use of an x-prefix option > is supported as a stable interface. A concur, that we don't have to declare it as stable in help/doc, but we still have to document x-use-canonical-path-for-ramblock-id=off the so users would know how/when to use it in this particular case. > Alternatively, if the option is really stable for direct > use by users then we should commit to making it so by > removing the x-. Peter Maydell, I think Peter Krempa already explained/pointed to discussion why x-use-canonical-path-for-ramblock-id wasn't renamed. So as I see options are: 1) keep x- prefix declare it as stable both in doc and comments (like in this patch) add to commit message why we are keeping x- 2) keep x- prefix declare it as stable in comments only, keep doc changes to explaining how/when to use it add to commit message why we are keeping x- 3) rename/drop x- prefix and don't care about QEMU-5.0-5.2 (libvirt would use old syntax (-mem-path/mem-prealloc) for them which also leads to => no virtiofs as it needs shared RAM that new syntax with backend provides for main RAM) Which one is acceptable to you? > thanks > -- PMM >
diff --git a/backends/hostmem.c b/backends/hostmem.c index 4bde00e8e7..6aaeefd0cf 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -497,6 +497,16 @@ host_memory_backend_class_init(ObjectClass *oc, void *data) host_memory_backend_get_share, host_memory_backend_set_share); object_class_property_set_description(oc, "share", "Mark the memory as private to QEMU or shared"); + /* + * Do not delete/rename option. This option must be considered stable + * (as if it didn't have the 'x-' prefix including deprecation period) as + * long as 4.0 and older machine types exists. + * Option will be used by upper layers to override (disable) canonical path + * for ramblock-id set by compat properties on old machine types ( <= 4.0), + * to keep migration working when backend is used for main RAM with + * -machine memory-backend= option (main RAM historically used prefix-less + * ramblock-id). + */ object_class_property_add_bool(oc, "x-use-canonical-path-for-ramblock-id", host_memory_backend_get_use_canonical_path, host_memory_backend_set_use_canonical_path); diff --git a/qemu-options.hx b/qemu-options.hx index 459c916d3d..22aa267c30 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -35,7 +35,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" " nvdimm=on|off controls NVDIMM support (default=off)\n" " memory-encryption=@var{} memory encryption object to use (default=none)\n" - " hmat=on|off controls ACPI HMAT support (default=off)\n", + " hmat=on|off controls ACPI HMAT support (default=off)\n" + " memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n", QEMU_ARCH_ALL) SRST ``-machine [type=]name[,prop=value[,...]]`` @@ -96,6 +97,31 @@ SRST ``hmat=on|off`` Enables or disables ACPI Heterogeneous Memory Attribute Table (HMAT) support. The default is off. + + ``memory-backend='id'`` + An alternative to legacy ``-mem-path`` and ``mem-prealloc`` options. + Allows to use a memory backend as main RAM. + + For example: + :: + -object memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on + -machine memory-backend=pc.ram + -m 512M + + Migration compatibility note: + a) as backend id one shall use value of 'default-ram-id', advertised by + machine type (available via ``query-machines`` QMP command) + b) for machine types 4.0 and older, user shall + use ``x-use-canonical-path-for-ramblock-id=on`` backend option + (this option must be considered stable, as if it didn't have the 'x-' + prefix including deprecation period, as long as 4.0 and older machine + types exists), + if migration to/from old QEMU (<5.0) is expected. + For example: + :: + -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=on + -machine memory-backend=pc.ram + -m 512M ERST HXCOMM Deprecated by -machine
Add documentation for '-machine memory-backend' CLI option and how to use it. And document that x-use-canonical-path-for-ramblock-id, is considered to be stable to make sure it won't go away by accident. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v2: - add doc that x-use-canonical-path-for-ramblock-id is considered stable, (Peter Krempa <pkrempa@redhat.com>) --- backends/hostmem.c | 10 ++++++++++ qemu-options.hx | 28 +++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-)