Message ID | 20220118150712.139953-2-david@redhat.com |
---|---|
State | New |
Headers | show |
Series | virtio-mem: Handle preallocation with migration | expand |
* David Hildenbrand (david@redhat.com) wrote: > "prealloc=on" for the memory backend does not work as expected, as > virtio-mem will simply discard all preallocated memory immediately again. > In the best case, it's an expensive NOP. In the worst case, it's an > unexpected allocation error. > > Instead, "prealloc=on" should be specified for the virtio-mem device only, > such that virtio-mem will try preallocating memory before plugging > memory dynamically to the guest. > > Signed-off-by: David Hildenbrand <david@redhat.com> Why is this a warning rather than an error that stops creation of the device? Dave > --- > hw/virtio/virtio-mem.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > index 04c223b0c9..6c337db0a7 100644 > --- a/hw/virtio/virtio-mem.c > +++ b/hw/virtio/virtio-mem.c > @@ -765,6 +765,13 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp) > return; > } > > + if (vmem->memdev->prealloc) { > + warn_report("'%s' property specifies a memdev with preallocation" > + " enabled: %s. Instead, specify 'prealloc=on' for the" > + " virtio-mem device. ", VIRTIO_MEM_MEMDEV_PROP, > + object_get_canonical_path_component(OBJECT(vmem->memdev))); > + } > + > if ((nb_numa_nodes && vmem->node >= nb_numa_nodes) || > (!nb_numa_nodes && vmem->node)) { > error_setg(errp, "'%s' property has value '%" PRIu32 "', which exceeds" > -- > 2.34.1 >
On 25.01.22 12:19, Dr. David Alan Gilbert wrote: > * David Hildenbrand (david@redhat.com) wrote: >> "prealloc=on" for the memory backend does not work as expected, as >> virtio-mem will simply discard all preallocated memory immediately again. >> In the best case, it's an expensive NOP. In the worst case, it's an >> unexpected allocation error. >> >> Instead, "prealloc=on" should be specified for the virtio-mem device only, >> such that virtio-mem will try preallocating memory before plugging >> memory dynamically to the guest. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> > > Why is this a warning rather than an error that stops creation of the > device? No strong opinion, an error might actually be better.
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index 04c223b0c9..6c337db0a7 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -765,6 +765,13 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp) return; } + if (vmem->memdev->prealloc) { + warn_report("'%s' property specifies a memdev with preallocation" + " enabled: %s. Instead, specify 'prealloc=on' for the" + " virtio-mem device. ", VIRTIO_MEM_MEMDEV_PROP, + object_get_canonical_path_component(OBJECT(vmem->memdev))); + } + if ((nb_numa_nodes && vmem->node >= nb_numa_nodes) || (!nb_numa_nodes && vmem->node)) { error_setg(errp, "'%s' property has value '%" PRIu32 "', which exceeds"
"prealloc=on" for the memory backend does not work as expected, as virtio-mem will simply discard all preallocated memory immediately again. In the best case, it's an expensive NOP. In the worst case, it's an unexpected allocation error. Instead, "prealloc=on" should be specified for the virtio-mem device only, such that virtio-mem will try preallocating memory before plugging memory dynamically to the guest. Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/virtio/virtio-mem.c | 7 +++++++ 1 file changed, 7 insertions(+)