Re: [PATCH v2 05/13] conf: Introduce virtio-mem <memory/> model

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 18, 2021 at 14:31:00 +0100, Michal Privoznik wrote:
> The virtio-mem is paravirtualized mechanism of adding/removing
> memory to/from a VM. A virtio-mem-pci device is split into blocks
> of equal size which are then exposed (all or only a requested
> portion of them) to the guest kernel to use as regular memory.
> Therefore, the device has two important attributes:
> 
>   1) block-size, which defines the size of a block
>   2) requested-size, which defines how much memory (in bytes)
>      is the device requested to expose to the guest.
> 
> The 'block-size' is configured on command line and immutable
> throughout device's lifetime. The 'requested-size' can be set on
> the command line too, but also is adjustable via monitor. In
> fact, that is how management software places its requests to
> change the memory allocation. If it wants to give more memory to
> the guest it changes 'requested-size' to a bigger value, and if it
> wants to shrink guest memory it changes the 'requested-size' to a
> smaller value. Note, value of zero means that guest should
> release all memory offered by the device. Of course, guest has to
> cooperate. Therefore, there is a third attribute 'size' which is
> read only and reflects how much memory the guest still has. This
> can be different to 'requested-size', obviously. Because of name
> clash, I've named it 'actualsize' and it is dealt with in future
> commits (it is a runtime information anyway).
> 
> In the backend, memory for virtio-mem is backed by usual objects:
> memory-backend-{ram,file,memfd} and their size puts the cap on
> the amount of memory that a virtio-mem device can offer to a
> guest. But we are already able to express this info using <size/>
> under <target/>.
> 
> Therefore, we need only two more elements to cover 'block-size'
> and 'requested-size' attributes. This is the XML I've came up
> with:
> 
>   <memory model='virtio-mem'>
>     <source>
>       <nodemask>1-3</nodemask>
>       <pagesize unit='KiB'>2048</pagesize>
>     </source>
>     <target>
>       <size unit='KiB'>2097152</size>
>       <node>0</node>
>       <block unit='KiB'>2048</block>
>       <requested unit='KiB'>1048576</requested>
>     </target>
>     <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
>   </memory>
> 
> I hope by now it is obvious that:
> 
>   1) 'requested-size' must be an integer multiple of
>      'block-size', and
>   2) virtio-mem-pci device goes onto PCI bus and thus needs PCI
>      address.
> 
> Then there is a limitation that the minimal 'block-size' is
> transparent huge page size (I'll leave this without explanation).
> 
> Since now we have (possibly) two or more devices that allow
> memory inflation/deflation and accounting for all of them (and
> thus keeping <currentMemory/> updated) might be hard. Therefore,
> I'm deliberately forbidding memballoon. It's okay - virtio-mem is
> superior to memballoon anyway. We can always reevaluate later.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  docs/formatdomain.rst                         | 51 +++++++++++++--
>  docs/schemas/domaincommon.rng                 | 11 ++++
>  src/conf/domain_conf.c                        | 53 ++++++++++++++-
>  src/conf/domain_conf.h                        |  3 +
>  src/conf/domain_validate.c                    | 39 +++++++++++
>  src/qemu/qemu_alias.c                         |  1 +
>  src/qemu/qemu_command.c                       |  1 +
>  src/qemu/qemu_domain.c                        | 27 +++++++-
>  src/qemu/qemu_domain_address.c                | 38 ++++++++---
>  src/qemu/qemu_process.c                       |  2 +
>  src/qemu/qemu_validate.c                      | 22 ++++++-
>  src/security/security_apparmor.c              |  1 +
>  src/security/security_dac.c                   |  2 +
>  src/security/security_selinux.c               |  2 +
>  .../memory-hotplug-virtio-mem.xml             | 64 +++++++++++++++++++
>  ...emory-hotplug-virtio-mem.x86_64-latest.xml |  1 +
>  tests/qemuxml2xmltest.c                       |  1 +
>  17 files changed, 298 insertions(+), 21 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml
>  create mode 120000 tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 2587106191..05d359a32d 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst

[...]

> @@ -6935,6 +6937,16 @@ Example: automatically added device with KVM
>     </devices>
>     ...
>  
> +Example: automatically added device with QEMU/KVM and a ``virtio-mem`` device:
> +
> +::
> +
> +   ...
> +   <devices>

The example is missing the 'virtio-mem' device which causes the balloon
to be disabled.


> +     <memballoon model='none'/>
> +   </devices>
> +   ...
> +
>  Example: manually added device with static PCI slot 2 requested
>  
>  ::


[...]

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 541d592bbe..26b9b5583e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3679,10 +3679,21 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
>      }
>  
>      if (addDefaultMemballoon && !def->memballoon) {
> -        virDomainMemballoonDefPtr memballoon;
> -        memballoon = g_new0(virDomainMemballoonDef, 1);
> +        virDomainMemballoonDefPtr memballoon = g_new0(virDomainMemballoonDef, 1);
> +        size_t i;
>  
> -        memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO;
> +        /* To simplify virtio-mem implementation, memballoon has to be turned
> +         * off if domain has a virtio-mem device. See
> +         * qemuValidateDomainDeviceDefMemory() for more details. */
> +        for (i = 0; i < def->nmems; i++) {
> +            if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM)
> +                break;
> +        }
> +
> +        if (i == def->nmems)
> +            memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO;
> +        else
> +            memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE;
>          def->memballoon = memballoon;
>      }

This might be a point of contention. I can see why we'd want to do this
but people might have other ideas. Preferably I'd like danpb's input.

I think the best approach will be to not hide it into the massive patch
adding the boring XML bits but split it separately.


[...]

> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 2541ae856a..6150083251 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c

[...]

> @@ -4688,6 +4689,25 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem,
>          }
>          break;
>  
> +    case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
> +        /* Accounting balloon and virtio-mem is hard. We have plenty of APIs
> +         * which take balloon from QEMU and report it to users. We would have
> +         * to change all that and account for virtio-mem actual size.  Also,
> +         * virtio-mem is supposed to be replacement for balloon. Disable
> +         * coexistence of these two for now. */
> +        if (virDomainDefHasMemballoon(def)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("virtio-mem is not supported with memory balloon"));
> +            return -1;
> +        }
> +
> +        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("virtio-mem isn't supported by this QEMU binary"));
> +            return -1;
> +        }
> +        break;

And this one too. I agree with this hunk as is, but not in this commit.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux