| Message ID | tencent_C7CEA36B3AB3971A278DFD71F2BBCFF88906@qq.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Chardev: delete the QemuOpts reserved in vm_config_groups | expand |
Hi On Sat, Nov 6, 2021 at 10:41 PM ぷ风过无痕?? <787738581@qq.com> wrote: > > We have found that qemu cannot plug in the previously unplugged device. > start qemu with the following command: > ./qemu-system-x86_64 -enable-kvm -m 8192 -smp 16 \ > -object memory-backend-file,id=mem,size=8192M,mem-path=/dev/hugepages,share=on \ > -numa node,memdev=mem -mem-prealloc \ > ... > -chardev socket,id=drive-virtio-disk1,path=/var/run/spdk/vhost_blk_socket-30dcf467-486a-45cf-b754-093bf5cf24d1,reconnect=10 \ > -device vhost-user-blk-pci,chardev=drive-virtio-disk1,num-queues=1,bus=pci.0,addr=0x5,id=virtio-disk1 \ > ... > > (qemu) info chardev > serial0: filename=pty:/dev/pts/0 > drive-virtio-disk1: filename=unix: > charmonitor: > filename=unix:/usr/local/var/lib/libvirt/qemu/domain-55-e59039db-d576-40db-a/monitor.sock,server > (qemu) device_del virtio-disk1 > (qemu) chardev-remove drive-virtio-disk1 > (qemu) info chardev > serial0: filename=pty:/dev/pts/0 > charmonitor: > filename=unix:/usr/local/var/lib/libvirt/qemu/domain-55-e59039db-d576-40db-a/monitor.sock,server > (qemu) chardev-add socket,id=drive-virtio-disk1,path=/var/run/spdk/vhost_blk_socket-30dcf467-486a-45cf-b754-093bf5cf24d1,reconnect=10 > device failed: Duplicate ID 'drive-virtio-disk1' for chardev Error: Parsing chardev args failed > > The root cause of this issue is that the QemuOpts is still reserved in vm_config_groups list, > so qemu_opts_create will fail with the error above. So the QemuOpts should be removed when > invoking qmp_chardev_remove. > > > Signed-off-by: Zhoujian Yu <windyu@tencent.com> > Reviewed-by: Lei Wang <kaierwang@tencent.com> > Reviewed-by: Xun Ni <richardni@tencent.com> > Reviewed-by: Zhigang Lu <tonnylu@tencent.com> > --- > chardev/char.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/chardev/char.c b/chardev/char.c > index 398f09d..d2087bd 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -37,6 +37,7 @@ > #include "qemu/help_option.h" > #include "qemu/module.h" > #include "qemu/option.h" > +#include "qemu/option_int.h" > #include "qemu/id.h" > #include "qemu/coroutine.h" > #include "qemu/yank.h" > @@ -1159,6 +1160,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > void qmp_chardev_remove(const char *id, Error **errp) > { > Chardev *chr; > + QemuOpts *opts; > > chr = qemu_chr_find(id); > if (chr == NULL) { > @@ -1175,6 +1177,10 @@ void qmp_chardev_remove(const char *id, Error **errp) > return; > } > object_unparent(OBJECT(chr)); > + /* delete the opts reserved in vm_config_groups list. */ > + opts = qemu_opts_find(qemu_find_opts("chardev"), id); > + if (opts) > + qemu_opts_del(opts); That seems reasonable to me, Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > } > > void qmp_chardev_send_break(const char *id, Error **errp) > -- > 1.8.3.1
Hi Lureau,
You say that hot-add failare is reasonable after hot-remove? I think it is a clear issue.
>Hi
On Sat, Nov 6, 2021 at 10:41 PM ぷ风过无痕?? <787738581@qq.com> wrote:
>
> We have found that qemu cannot plug in the previously unplugged device.
> start qemu with the following command:
> ./qemu-system-x86_64 -enable-kvm -m 8192 -smp 16 \
> -object memory-backend-file,id=mem,size=8192M,mem-path=/dev/hugepages,share=on \
> -numa node,memdev=mem -mem-prealloc \
> ...
> -chardev socket,id=drive-virtio-disk1,path=/var/run/spdk/vhost_blk_socket-30dcf467-486a-45cf-b754-093bf5cf24d1,reconnect=10 \
> -device vhost-user-blk-pci,chardev=drive-virtio-disk1,num-queues=1,bus=pci.0,addr=0x5,id=virtio-disk1 \
> ...
>
> (qemu) info chardev
> serial0: filename=pty:/dev/pts/0
> drive-virtio-disk1: filename=unix:
> charmonitor:
> filename=unix:/usr/local/var/lib/libvirt/qemu/domain-55-e59039db-d576-40db-a/monitor.sock,server
> (qemu) device_del virtio-disk1
> (qemu) chardev-remove drive-virtio-disk1
> (qemu) info chardev
> serial0: filename=pty:/dev/pts/0
> charmonitor:
> filename=unix:/usr/local/var/lib/libvirt/qemu/domain-55-e59039db-d576-40db-a/monitor.sock,server
> (qemu) chardev-add socket,id=drive-virtio-disk1,path=/var/run/spdk/vhost_blk_socket-30dcf467-486a-45cf-b754-093bf5cf24d1,reconnect=10
> device failed: Duplicate ID 'drive-virtio-disk1' for chardev Error: Parsing chardev args failed
>
> The root cause of this issue is that the QemuOpts is still reserved in vm_config_groups list,
> so qemu_opts_create will fail with the error above. So the QemuOpts should be removed when
> invoking qmp_chardev_remove.
>
>
> Signed-off-by: Zhoujian Yu <windyu@tencent.com>
> Reviewed-by: Lei Wang <kaierwang@tencent.com>
> Reviewed-by: Xun Ni <richardni@tencent.com>
> Reviewed-by: Zhigang Lu <tonnylu@tencent.com>
> ---
> chardev/char.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 398f09d..d2087bd 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -37,6 +37,7 @@
> #include "qemu/help_option.h"
> #include "qemu/module.h"
> #include "qemu/option.h"
> +#include "qemu/option_int.h"
> #include "qemu/id.h"
> #include "qemu/coroutine.h"
> #include "qemu/yank.h"
> @@ -1159,6 +1160,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
> void qmp_chardev_remove(const char *id, Error **errp)
> {
> Chardev *chr;
> + QemuOpts *opts;
>
> chr = qemu_chr_find(id);
> if (chr == NULL) {
> @@ -1175,6 +1177,10 @@ void qmp_chardev_remove(const char *id, Error **errp)
> return;
> }
> object_unparent(OBJECT(chr));
> + /* delete the opts reserved in vm_config_groups list. */
> + opts = qemu_opts_find(qemu_find_opts("chardev"), id);
> + if (opts)
> + qemu_opts_del(opts);
That seems reasonable to me,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> }
>
> void qmp_chardev_send_break(const char *id, Error **errp)
> --
> 1.8.3.1
PING
thanks, Wind Yu
Hi Lureau,
You say that hot-add failare is reasonable after hot-remove? I think it is a clear issue.
>Hi
On Sat, Nov 6, 2021 at 10:41 PM ぷ风过无痕?? <787738581@qq.com> wrote:
>
> We have found that qemu cannot plug in the previously unplugged device.
> start qemu with the following command:
> ./qemu-system-x86_64 -enable-kvm -m 8192 -smp 16 \
> -object memory-backend-file,id=mem,size=8192M,mem-path=/dev/hugepages,share=on \
> -numa node,memdev=mem -mem-prealloc \
> ...
> -chardev socket,id=drive-virtio-disk1,path=/var/run/spdk/vhost_blk_socket-30dcf467-486a-45cf-b754-093bf5cf24d1,reconnect=10 \
> -device vhost-user-blk-pci,chardev=drive-virtio-disk1,num-queues=1,bus=pci.0,addr=0x5,id=virtio-disk1 \
> ...
>
> (qemu) info chardev
> serial0: filename=pty:/dev/pts/0
> drive-virtio-disk1: filename=unix:
> charmonitor:
> filename=unix:/usr/local/var/lib/libvirt/qemu/domain-55-e59039db-d576-40db-a/monitor.sock,server
> (qemu) device_del virtio-disk1
> (qemu) chardev-remove drive-virtio-disk1
> (qemu) info chardev
> serial0: filename=pty:/dev/pts/0
> charmonitor:
> filename=unix:/usr/local/var/lib/libvirt/qemu/domain-55-e59039db-d576-40db-a/monitor.sock,server
> (qemu) chardev-add socket,id=drive-virtio-disk1,path=/var/run/spdk/vhost_blk_socket-30dcf467-486a-45cf-b754-093bf5cf24d1,reconnect=10
> device failed: Duplicate ID 'drive-virtio-disk1' for chardev Error: Parsing chardev args failed
>
> The root cause of this issue is that the QemuOpts is still reserved in vm_config_groups list,
> so qemu_opts_create will fail with the error above. So the QemuOpts should be removed when
> invoking qmp_chardev_remove.
>
>
> Signed-off-by: Zhoujian Yu <windyu@tencent.com>
> Reviewed-by: Lei Wang <kaierwang@tencent.com>
> Reviewed-by: Xun Ni <richardni@tencent.com>
> Reviewed-by: Zhigang Lu <tonnylu@tencent.com>
> ---
> chardev/char.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 398f09d..d2087bd 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -37,6 +37,7 @@
> #include "qemu/help_option.h"
> #include "qemu/module.h"
> #include "qemu/option.h"
> +#include "qemu/option_int.h"
> #include "qemu/id.h"
> #include "qemu/coroutine.h"
> #include "qemu/yank.h"
> @@ -1159,6 +1160,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
> void qmp_chardev_remove(const char *id, Error **errp)
> {
> Chardev *chr;
> + QemuOpts *opts;
>
> chr = qemu_chr_find(id);
> if (chr == NULL) {
> @@ -1175,6 +1177,10 @@ void qmp_chardev_remove(const char *id, Error **errp)
> return;
> }
> object_unparent(OBJECT(chr));
> + /* delete the opts reserved in vm_config_groups list. */
> + opts = qemu_opts_find(qemu_find_opts("chardev"), id);
> + if (opts)
> + qemu_opts_del(opts);
That seems reasonable to me,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> }
>
> void qmp_chardev_send_break(const char *id, Error **errp)
> --
> 1.8.3.1
On 11/8/21 09:46, Marc-André Lureau wrote: >> + QemuOpts *opts; >> >> chr = qemu_chr_find(id); >> if (chr == NULL) { >> @@ -1175,6 +1177,10 @@ void qmp_chardev_remove(const char *id, Error **errp) >> return; >> } >> object_unparent(OBJECT(chr)); >> + /* delete the opts reserved in vm_config_groups list. */ >> + opts = qemu_opts_find(qemu_find_opts("chardev"), id); >> + if (opts) >> + qemu_opts_del(opts); > That seems reasonable to me, > Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com> I think it should be done instead in an unparent function, i.e. with something like "oc->unparent = chr_unparent;" in char_class_init. chr_unparent does the qemu_opts_find/qemu_opts_del. Thanks, Paolo
OK, thank you Paolo! I will write a char_unparent as your comment.
Wind Yu
------------------ 原始邮件 ------------------
发件人: "Paolo Bonzini" <pbonzini@redhat.com>;
发送时间: 2021年11月19日(星期五) 下午5:27
收件人: "Marc-André Lureau"<marcandre.lureau@redhat.com>;"ぷ风过无痕??"<787738581@qq.com>;
抄送: "qemu-devel"<qemu-devel@nongnu.org>;
主题: Re: Chardev: delete the QemuOpts reserved in vm_config_groups
On 11/8/21 09:46, Marc-André Lureau wrote:
>> + QemuOpts *opts;
>>
>> chr = qemu_chr_find(id);
>> if (chr == NULL) {
>> @@ -1175,6 +1177,10 @@ void qmp_chardev_remove(const char *id, Error **errp)
>> return;
>> }
>> object_unparent(OBJECT(chr));
>> + /* delete the opts reserved in vm_config_groups list. */
>> + opts = qemu_opts_find(qemu_find_opts("chardev"), id);
>> + if (opts)
>> + qemu_opts_del(opts);
> That seems reasonable to me,
> Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com>
I think it should be done instead in an unparent function, i.e. with
something like "oc->unparent = chr_unparent;" in char_class_init.
chr_unparent does the qemu_opts_find/qemu_opts_del.
Thanks,
Paolo
diff --git a/chardev/char.c b/chardev/char.c index 398f09d..d2087bd 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -37,6 +37,7 @@ #include "qemu/help_option.h" #include "qemu/module.h" #include "qemu/option.h" +#include "qemu/option_int.h" #include "qemu/id.h" #include "qemu/coroutine.h" #include "qemu/yank.h" @@ -1159,6 +1160,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, void qmp_chardev_remove(const char *id, Error **errp) { Chardev *chr; + QemuOpts *opts; chr = qemu_chr_find(id); if (chr == NULL) { @@ -1175,6 +1177,10 @@ void qmp_chardev_remove(const char *id, Error **errp) return; } object_unparent(OBJECT(chr)); + /* delete the opts reserved in vm_config_groups list. */ + opts = qemu_opts_find(qemu_find_opts("chardev"), id); + if (opts) + qemu_opts_del(opts); } void qmp_chardev_send_break(const char *id, Error **errp)