| Message ID | 20200115224025.538368-3-jusual@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-balloon: Reject qmp_balloon during hot-unplug | expand |
On 15.01.20 23:40, Julia Suvorova wrote: > Hot-unplug takes some time due to communication with the guest. > Do not change the device while freeing up resources. > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > --- > balloon.c | 2 +- > hw/virtio/virtio-balloon.c | 9 ++++++++- > include/sysemu/balloon.h | 2 +- > 3 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/balloon.c b/balloon.c > index f104b42961..998ec53a0f 100644 > --- a/balloon.c > +++ b/balloon.c > @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) > } > > trace_balloon_event(balloon_opaque, target); > - balloon_event_fn(balloon_opaque, target); > + balloon_event_fn(balloon_opaque, target, errp); > } > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 57f3b9f22d..0fa4e4454b 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > VIRTIO_BALLOON_PFN_SHIFT); > } > > -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > + Error **errp) > { > + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); > VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > ram_addr_t vm_ram_size = get_current_ram_size(); > > + if (bus_dev && bus_dev->pending_deleted_event) { > + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); > + return; > + } > + How exactly does this help? The guest is free to inflate/deflate whenever it wants.
On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: > > On 15.01.20 23:40, Julia Suvorova wrote: > > Hot-unplug takes some time due to communication with the guest. > > Do not change the device while freeing up resources. > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > --- > > balloon.c | 2 +- > > hw/virtio/virtio-balloon.c | 9 ++++++++- > > include/sysemu/balloon.h | 2 +- > > 3 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/balloon.c b/balloon.c > > index f104b42961..998ec53a0f 100644 > > --- a/balloon.c > > +++ b/balloon.c > > @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) > > } > > > > trace_balloon_event(balloon_opaque, target); > > - balloon_event_fn(balloon_opaque, target); > > + balloon_event_fn(balloon_opaque, target, errp); > > } > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > > index 57f3b9f22d..0fa4e4454b 100644 > > --- a/hw/virtio/virtio-balloon.c > > +++ b/hw/virtio/virtio-balloon.c > > @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > > VIRTIO_BALLOON_PFN_SHIFT); > > } > > > > -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > > +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > > + Error **errp) > > { > > + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); > > VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > ram_addr_t vm_ram_size = get_current_ram_size(); > > > > + if (bus_dev && bus_dev->pending_deleted_event) { > > + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); > > + return; > > + } > > + > > How exactly does this help? The guest is free to inflate/deflate > whenever it wants. Guest is aware of hot-unplug start, and virtio driver should not initiate any operations. This patch just restricts issuing commands from qmp monitor. Best regards, Julia Suvorova.
On 23.01.20 15:08, Julia Suvorova wrote: > On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: >> >> On 15.01.20 23:40, Julia Suvorova wrote: >>> Hot-unplug takes some time due to communication with the guest. >>> Do not change the device while freeing up resources. >>> >>> Signed-off-by: Julia Suvorova <jusual@redhat.com> >>> --- >>> balloon.c | 2 +- >>> hw/virtio/virtio-balloon.c | 9 ++++++++- >>> include/sysemu/balloon.h | 2 +- >>> 3 files changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/balloon.c b/balloon.c >>> index f104b42961..998ec53a0f 100644 >>> --- a/balloon.c >>> +++ b/balloon.c >>> @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) >>> } >>> >>> trace_balloon_event(balloon_opaque, target); >>> - balloon_event_fn(balloon_opaque, target); >>> + balloon_event_fn(balloon_opaque, target, errp); >>> } >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c >>> index 57f3b9f22d..0fa4e4454b 100644 >>> --- a/hw/virtio/virtio-balloon.c >>> +++ b/hw/virtio/virtio-balloon.c >>> @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) >>> VIRTIO_BALLOON_PFN_SHIFT); >>> } >>> >>> -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) >>> +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, >>> + Error **errp) >>> { >>> + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); >>> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); >>> VirtIODevice *vdev = VIRTIO_DEVICE(dev); >>> ram_addr_t vm_ram_size = get_current_ram_size(); >>> >>> + if (bus_dev && bus_dev->pending_deleted_event) { >>> + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); >>> + return; >>> + } >>> + >> >> How exactly does this help? The guest is free to inflate/deflate >> whenever it wants. > > Guest is aware of hot-unplug start, and virtio driver should not > initiate any operations. This patch just restricts issuing commands > from qmp monitor. Why shouldn't the guest driver inflate/deflate while memory hotplug is going on? Simple balloon compaction in a Linux guest -> deflate/inflate triggered in the hypervisor.
On Thu, Jan 23, 2020 at 3:17 PM David Hildenbrand <david@redhat.com> wrote: > > On 23.01.20 15:08, Julia Suvorova wrote: > > On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: > >> > >> On 15.01.20 23:40, Julia Suvorova wrote: > >>> Hot-unplug takes some time due to communication with the guest. > >>> Do not change the device while freeing up resources. > >>> > >>> Signed-off-by: Julia Suvorova <jusual@redhat.com> > >>> --- > >>> balloon.c | 2 +- > >>> hw/virtio/virtio-balloon.c | 9 ++++++++- > >>> include/sysemu/balloon.h | 2 +- > >>> 3 files changed, 10 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/balloon.c b/balloon.c > >>> index f104b42961..998ec53a0f 100644 > >>> --- a/balloon.c > >>> +++ b/balloon.c > >>> @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) > >>> } > >>> > >>> trace_balloon_event(balloon_opaque, target); > >>> - balloon_event_fn(balloon_opaque, target); > >>> + balloon_event_fn(balloon_opaque, target, errp); > >>> } > >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > >>> index 57f3b9f22d..0fa4e4454b 100644 > >>> --- a/hw/virtio/virtio-balloon.c > >>> +++ b/hw/virtio/virtio-balloon.c > >>> @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > >>> VIRTIO_BALLOON_PFN_SHIFT); > >>> } > >>> > >>> -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > >>> +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > >>> + Error **errp) > >>> { > >>> + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); > >>> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > >>> VirtIODevice *vdev = VIRTIO_DEVICE(dev); > >>> ram_addr_t vm_ram_size = get_current_ram_size(); > >>> > >>> + if (bus_dev && bus_dev->pending_deleted_event) { > >>> + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); > >>> + return; > >>> + } > >>> + > >> > >> How exactly does this help? The guest is free to inflate/deflate > >> whenever it wants. > > > > Guest is aware of hot-unplug start, and virtio driver should not > > initiate any operations. This patch just restricts issuing commands > > from qmp monitor. > > Why shouldn't the guest driver inflate/deflate while memory hotplug is > going on? > > Simple balloon compaction in a Linux guest -> deflate/inflate triggered > in the hypervisor. QEMU crashes if inflate happens after powering-off PCI slot. Guest is unable to interact with virtio-balloon device then, driver is unloaded. But inflate can still happen if initiated from qmp. Best regards, Julia Suvorova.
On 23.01.20 16:46, Julia Suvorova wrote: > On Thu, Jan 23, 2020 at 3:17 PM David Hildenbrand <david@redhat.com> wrote: >> >> On 23.01.20 15:08, Julia Suvorova wrote: >>> On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: >>>> >>>> On 15.01.20 23:40, Julia Suvorova wrote: >>>>> Hot-unplug takes some time due to communication with the guest. >>>>> Do not change the device while freeing up resources. >>>>> >>>>> Signed-off-by: Julia Suvorova <jusual@redhat.com> >>>>> --- >>>>> balloon.c | 2 +- >>>>> hw/virtio/virtio-balloon.c | 9 ++++++++- >>>>> include/sysemu/balloon.h | 2 +- >>>>> 3 files changed, 10 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/balloon.c b/balloon.c >>>>> index f104b42961..998ec53a0f 100644 >>>>> --- a/balloon.c >>>>> +++ b/balloon.c >>>>> @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) >>>>> } >>>>> >>>>> trace_balloon_event(balloon_opaque, target); >>>>> - balloon_event_fn(balloon_opaque, target); >>>>> + balloon_event_fn(balloon_opaque, target, errp); >>>>> } >>>>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c >>>>> index 57f3b9f22d..0fa4e4454b 100644 >>>>> --- a/hw/virtio/virtio-balloon.c >>>>> +++ b/hw/virtio/virtio-balloon.c >>>>> @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) >>>>> VIRTIO_BALLOON_PFN_SHIFT); >>>>> } >>>>> >>>>> -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) >>>>> +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, >>>>> + Error **errp) >>>>> { >>>>> + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); >>>>> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); >>>>> VirtIODevice *vdev = VIRTIO_DEVICE(dev); >>>>> ram_addr_t vm_ram_size = get_current_ram_size(); >>>>> >>>>> + if (bus_dev && bus_dev->pending_deleted_event) { >>>>> + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); >>>>> + return; >>>>> + } >>>>> + >>>> >>>> How exactly does this help? The guest is free to inflate/deflate >>>> whenever it wants. >>> >>> Guest is aware of hot-unplug start, and virtio driver should not >>> initiate any operations. This patch just restricts issuing commands >>> from qmp monitor. >> >> Why shouldn't the guest driver inflate/deflate while memory hotplug is >> going on? >> >> Simple balloon compaction in a Linux guest -> deflate/inflate triggered >> in the hypervisor. > > QEMU crashes if inflate happens after powering-off PCI slot. Guest is > unable to interact with virtio-balloon device then, driver is > unloaded. But inflate can still happen if initiated from qmp. Ah guest can inflate/deflate whenever it wants. What am I missing? IMHO, the right approach is to discard inflate/deflate requests on these areas instead.
On Thu, Jan 23, 2020 at 04:46:18PM +0100, Julia Suvorova wrote: > On Thu, Jan 23, 2020 at 3:17 PM David Hildenbrand <david@redhat.com> wrote: > > > > On 23.01.20 15:08, Julia Suvorova wrote: > > > On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: > > >> > > >> On 15.01.20 23:40, Julia Suvorova wrote: > > >>> Hot-unplug takes some time due to communication with the guest. > > >>> Do not change the device while freeing up resources. > > >>> > > >>> Signed-off-by: Julia Suvorova <jusual@redhat.com> > > >>> --- > > >>> balloon.c | 2 +- > > >>> hw/virtio/virtio-balloon.c | 9 ++++++++- > > >>> include/sysemu/balloon.h | 2 +- > > >>> 3 files changed, 10 insertions(+), 3 deletions(-) > > >>> > > >>> diff --git a/balloon.c b/balloon.c > > >>> index f104b42961..998ec53a0f 100644 > > >>> --- a/balloon.c > > >>> +++ b/balloon.c > > >>> @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) > > >>> } > > >>> > > >>> trace_balloon_event(balloon_opaque, target); > > >>> - balloon_event_fn(balloon_opaque, target); > > >>> + balloon_event_fn(balloon_opaque, target, errp); > > >>> } > > >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > > >>> index 57f3b9f22d..0fa4e4454b 100644 > > >>> --- a/hw/virtio/virtio-balloon.c > > >>> +++ b/hw/virtio/virtio-balloon.c > > >>> @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > > >>> VIRTIO_BALLOON_PFN_SHIFT); > > >>> } > > >>> > > >>> -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > > >>> +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > > >>> + Error **errp) > > >>> { > > >>> + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); > > >>> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > > >>> VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > >>> ram_addr_t vm_ram_size = get_current_ram_size(); > > >>> > > >>> + if (bus_dev && bus_dev->pending_deleted_event) { > > >>> + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); > > >>> + return; > > >>> + } > > >>> + > > >> > > >> How exactly does this help? The guest is free to inflate/deflate > > >> whenever it wants. > > > > > > Guest is aware of hot-unplug start, and virtio driver should not > > > initiate any operations. This patch just restricts issuing commands > > > from qmp monitor. > > > > Why shouldn't the guest driver inflate/deflate while memory hotplug is > > going on? > > > > Simple balloon compaction in a Linux guest -> deflate/inflate triggered > > in the hypervisor. > > QEMU crashes if inflate happens after powering-off PCI slot. Guest is > unable to interact with virtio-balloon device then, driver is > unloaded. But inflate can still happen if initiated from qmp. > > Best regards, Julia Suvorova. Hot-unplug in progress is probably a hack - we should prevent access when device is powered off. Also, it would appear that have_balloon is the correct place for this kind of check.
On Mon, Jan 27, 2020 at 11:54 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Thu, Jan 23, 2020 at 04:46:18PM +0100, Julia Suvorova wrote: > > On Thu, Jan 23, 2020 at 3:17 PM David Hildenbrand <david@redhat.com> wrote: > > > > > > On 23.01.20 15:08, Julia Suvorova wrote: > > > > On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: > > > >> > > > >> On 15.01.20 23:40, Julia Suvorova wrote: > > > >>> Hot-unplug takes some time due to communication with the guest. > > > >>> Do not change the device while freeing up resources. > > > >>> > > > >>> Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > >>> --- > > > >>> balloon.c | 2 +- > > > >>> hw/virtio/virtio-balloon.c | 9 ++++++++- > > > >>> include/sysemu/balloon.h | 2 +- > > > >>> 3 files changed, 10 insertions(+), 3 deletions(-) > > > >>> > > > >>> diff --git a/balloon.c b/balloon.c > > > >>> index f104b42961..998ec53a0f 100644 > > > >>> --- a/balloon.c > > > >>> +++ b/balloon.c > > > >>> @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) > > > >>> } > > > >>> > > > >>> trace_balloon_event(balloon_opaque, target); > > > >>> - balloon_event_fn(balloon_opaque, target); > > > >>> + balloon_event_fn(balloon_opaque, target, errp); > > > >>> } > > > >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > > > >>> index 57f3b9f22d..0fa4e4454b 100644 > > > >>> --- a/hw/virtio/virtio-balloon.c > > > >>> +++ b/hw/virtio/virtio-balloon.c > > > >>> @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > > > >>> VIRTIO_BALLOON_PFN_SHIFT); > > > >>> } > > > >>> > > > >>> -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > > > >>> +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > > > >>> + Error **errp) > > > >>> { > > > >>> + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); > > > >>> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > > > >>> VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > > >>> ram_addr_t vm_ram_size = get_current_ram_size(); > > > >>> > > > >>> + if (bus_dev && bus_dev->pending_deleted_event) { > > > >>> + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); > > > >>> + return; > > > >>> + } > > > >>> + > > > >> > > > >> How exactly does this help? The guest is free to inflate/deflate > > > >> whenever it wants. > > > > > > > > Guest is aware of hot-unplug start, and virtio driver should not > > > > initiate any operations. This patch just restricts issuing commands > > > > from qmp monitor. > > > > > > Why shouldn't the guest driver inflate/deflate while memory hotplug is > > > going on? > > > > > > Simple balloon compaction in a Linux guest -> deflate/inflate triggered > > > in the hypervisor. > > > > QEMU crashes if inflate happens after powering-off PCI slot. Guest is > > unable to interact with virtio-balloon device then, driver is > > unloaded. But inflate can still happen if initiated from qmp. > > > > Best regards, Julia Suvorova. > > Hot-unplug in progress is probably a hack - we should > prevent access when device is powered off. > Also, it would appear that have_balloon is the correct place for this > kind of check. You're right, it is a hack. I can add another handler to check if device is powered on, and check it in have_balloon(), In assumption that have_balloon means than we can use balloon when it's presented, it is fine. But it will get more complicated once hidden devices are here. Best regards, Julia Suvorova.
On Mon, Jan 27, 2020 at 03:53:06PM +0100, Julia Suvorova wrote: > On Mon, Jan 27, 2020 at 11:54 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Thu, Jan 23, 2020 at 04:46:18PM +0100, Julia Suvorova wrote: > > > On Thu, Jan 23, 2020 at 3:17 PM David Hildenbrand <david@redhat.com> wrote: > > > > > > > > On 23.01.20 15:08, Julia Suvorova wrote: > > > > > On Thu, Jan 16, 2020 at 1:36 PM David Hildenbrand <david@redhat.com> wrote: > > > > >> > > > > >> On 15.01.20 23:40, Julia Suvorova wrote: > > > > >>> Hot-unplug takes some time due to communication with the guest. > > > > >>> Do not change the device while freeing up resources. > > > > >>> > > > > >>> Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > > >>> --- > > > > >>> balloon.c | 2 +- > > > > >>> hw/virtio/virtio-balloon.c | 9 ++++++++- > > > > >>> include/sysemu/balloon.h | 2 +- > > > > >>> 3 files changed, 10 insertions(+), 3 deletions(-) > > > > >>> > > > > >>> diff --git a/balloon.c b/balloon.c > > > > >>> index f104b42961..998ec53a0f 100644 > > > > >>> --- a/balloon.c > > > > >>> +++ b/balloon.c > > > > >>> @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) > > > > >>> } > > > > >>> > > > > >>> trace_balloon_event(balloon_opaque, target); > > > > >>> - balloon_event_fn(balloon_opaque, target); > > > > >>> + balloon_event_fn(balloon_opaque, target, errp); > > > > >>> } > > > > >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > > > > >>> index 57f3b9f22d..0fa4e4454b 100644 > > > > >>> --- a/hw/virtio/virtio-balloon.c > > > > >>> +++ b/hw/virtio/virtio-balloon.c > > > > >>> @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > > > > >>> VIRTIO_BALLOON_PFN_SHIFT); > > > > >>> } > > > > >>> > > > > >>> -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > > > > >>> +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > > > > >>> + Error **errp) > > > > >>> { > > > > >>> + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); > > > > >>> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > > > > >>> VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > > > >>> ram_addr_t vm_ram_size = get_current_ram_size(); > > > > >>> > > > > >>> + if (bus_dev && bus_dev->pending_deleted_event) { > > > > >>> + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); > > > > >>> + return; > > > > >>> + } > > > > >>> + > > > > >> > > > > >> How exactly does this help? The guest is free to inflate/deflate > > > > >> whenever it wants. > > > > > > > > > > Guest is aware of hot-unplug start, and virtio driver should not > > > > > initiate any operations. This patch just restricts issuing commands > > > > > from qmp monitor. > > > > > > > > Why shouldn't the guest driver inflate/deflate while memory hotplug is > > > > going on? > > > > > > > > Simple balloon compaction in a Linux guest -> deflate/inflate triggered > > > > in the hypervisor. > > > > > > QEMU crashes if inflate happens after powering-off PCI slot. Guest is > > > unable to interact with virtio-balloon device then, driver is > > > unloaded. But inflate can still happen if initiated from qmp. > > > > > > Best regards, Julia Suvorova. > > > > Hot-unplug in progress is probably a hack - we should > > prevent access when device is powered off. > > Also, it would appear that have_balloon is the correct place for this > > kind of check. > > You're right, it is a hack. I can add another handler to check if > device is powered on, and check it in have_balloon(), > In assumption that have_balloon means than we can use balloon when > it's presented, it is fine. But it will get more complicated once > hidden devices are here. > > Best regards, Julia Suvorova. So thinking about it, all we need to do is make sure powered off devices don't send interrupts via msi or intx. Changing config space should be fine ... No?
diff --git a/balloon.c b/balloon.c index f104b42961..998ec53a0f 100644 --- a/balloon.c +++ b/balloon.c @@ -119,5 +119,5 @@ void qmp_balloon(int64_t target, Error **errp) } trace_balloon_event(balloon_opaque, target); - balloon_event_fn(balloon_opaque, target); + balloon_event_fn(balloon_opaque, target, errp); } diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 57f3b9f22d..0fa4e4454b 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -717,12 +717,19 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) VIRTIO_BALLOON_PFN_SHIFT); } -static void virtio_balloon_to_target(void *opaque, ram_addr_t target) +static void virtio_balloon_to_target(void *opaque, ram_addr_t target, + Error **errp) { + DeviceState *bus_dev = qdev_get_bus_device(DEVICE(opaque)); VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); VirtIODevice *vdev = VIRTIO_DEVICE(dev); ram_addr_t vm_ram_size = get_current_ram_size(); + if (bus_dev && bus_dev->pending_deleted_event) { + error_setg(errp, "Hot-unplug of %s is in progress", vdev->name); + return; + } + if (target > vm_ram_size) { target = vm_ram_size; } diff --git a/include/sysemu/balloon.h b/include/sysemu/balloon.h index aea0c44985..b3a09ca946 100644 --- a/include/sysemu/balloon.h +++ b/include/sysemu/balloon.h @@ -17,7 +17,7 @@ #include "exec/cpu-common.h" #include "qapi/qapi-types-misc.h" -typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target); +typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target, Error **errp); typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info); int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
Hot-unplug takes some time due to communication with the guest. Do not change the device while freeing up resources. Signed-off-by: Julia Suvorova <jusual@redhat.com> --- balloon.c | 2 +- hw/virtio/virtio-balloon.c | 9 ++++++++- include/sysemu/balloon.h | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-)