| Message ID | 1615529786-30763-4-git-send-email-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/16] virtio-net: calculating proper msix vectors on init | expand |
On 12/03/21 07:16, Jason Wang wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > When a network or network device is created from the command line or HMP, > QemuOpts ensures that the id passes the id_wellformed check. However, > QMP skips this: > > $ qemu-system-x86_64 -qmp stdio -S -nic user,id=123/456 > qemu-system-x86_64: -nic user,id=123/456: Parameter id expects an identifier > Identifiers consist of letters, digits, -, ., _, starting with a letter. > > $ qemu-system-x86_64 -qmp stdio -S > {"execute":"qmp_capabilities"} > {"return": {}} > {"execute":"netdev_add", "arguments": {"type": "user", "id": "123/456"}} > {"return": {}} > > After: > > $ qemu-system-x86_64 -qmp stdio -S > {"execute":"qmp_capabilities"} > {"return": {}} > {"execute":"netdev_add", "arguments": {"type": "user", "id": "123/456"}} > {"error": {"class": "GenericError", "desc": "Parameter "id" expects an identifier"}} > > Validity checks should be performed always at the bottom of the call chain, > because QMP skips all the steps above. Do this for the network subsystem. > > Cc: Jason Wang <jasowang@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > net/net.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/net/net.c b/net/net.c > index 9c784da..d36729f 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -44,6 +44,7 @@ > #include "qemu/cutils.h" > #include "qemu/config-file.h" > #include "qemu/ctype.h" > +#include "qemu/id.h" > #include "qemu/iov.h" > #include "qemu/qemu-print.h" > #include "qemu/main-loop.h" > @@ -1011,6 +1012,17 @@ static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp) > } > } > > + /* > + * The id for -net has already been checked by QemuOpts and > + * could be automatically generated, in which case it is not > + * well-formed by design. HMP and QMP only call us with > + * is_netdev == true. > + */ > + if (is_netdev && !id_wellformed(netdev->id)) { > + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); > + return -1; > + } > + > nc = qemu_find_netdev(netdev->id); > if (nc) { > error_setg(errp, "Duplicate ID '%s'", netdev->id); > Sorry, I sent v2 yesterday. This patch passed the tests at the time it was submitted, but now fails (because it does not work with -nic). Paolo
On 2021/3/12 4:44 下午, Paolo Bonzini wrote: > On 12/03/21 07:16, Jason Wang wrote: >> From: Paolo Bonzini <pbonzini@redhat.com> >> >> When a network or network device is created from the command line or >> HMP, >> QemuOpts ensures that the id passes the id_wellformed check. However, >> QMP skips this: >> >> $ qemu-system-x86_64 -qmp stdio -S -nic user,id=123/456 >> qemu-system-x86_64: -nic user,id=123/456: Parameter id expects an >> identifier >> Identifiers consist of letters, digits, -, ., _, starting with a >> letter. >> >> $ qemu-system-x86_64 -qmp stdio -S >> {"execute":"qmp_capabilities"} >> {"return": {}} >> {"execute":"netdev_add", "arguments": {"type": "user", "id": >> "123/456"}} >> {"return": {}} >> >> After: >> >> $ qemu-system-x86_64 -qmp stdio -S >> {"execute":"qmp_capabilities"} >> {"return": {}} >> {"execute":"netdev_add", "arguments": {"type": "user", "id": >> "123/456"}} >> {"error": {"class": "GenericError", "desc": "Parameter "id" >> expects an identifier"}} >> >> Validity checks should be performed always at the bottom of the call >> chain, >> because QMP skips all the steps above. Do this for the network >> subsystem. >> >> Cc: Jason Wang <jasowang@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> net/net.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/net/net.c b/net/net.c >> index 9c784da..d36729f 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -44,6 +44,7 @@ >> #include "qemu/cutils.h" >> #include "qemu/config-file.h" >> #include "qemu/ctype.h" >> +#include "qemu/id.h" >> #include "qemu/iov.h" >> #include "qemu/qemu-print.h" >> #include "qemu/main-loop.h" >> @@ -1011,6 +1012,17 @@ static int net_client_init1(const Netdev >> *netdev, bool is_netdev, Error **errp) >> } >> } >> + /* >> + * The id for -net has already been checked by QemuOpts and >> + * could be automatically generated, in which case it is not >> + * well-formed by design. HMP and QMP only call us with >> + * is_netdev == true. >> + */ >> + if (is_netdev && !id_wellformed(netdev->id)) { >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an >> identifier"); >> + return -1; >> + } >> + >> nc = qemu_find_netdev(netdev->id); >> if (nc) { >> error_setg(errp, "Duplicate ID '%s'", netdev->id); >> > > Sorry, I sent v2 yesterday. This patch passed the tests at the time > it was submitted, but now fails (because it does not work with -nic). I don't see that. But I add a fixup in the pull request: https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg04237.html If it doesn't make sense, I will drop this and send a new pull request next week. THanks > > Paolo > >
On 12/03/21 14:26, Jason Wang wrote: > > On 2021/3/12 4:44 下午, Paolo Bonzini wrote: >> On 12/03/21 07:16, Jason Wang wrote: >>> From: Paolo Bonzini <pbonzini@redhat.com> >>> >>> When a network or network device is created from the command line or >>> HMP, >>> QemuOpts ensures that the id passes the id_wellformed check. However, >>> QMP skips this: >>> >>> $ qemu-system-x86_64 -qmp stdio -S -nic user,id=123/456 >>> qemu-system-x86_64: -nic user,id=123/456: Parameter id expects an >>> identifier >>> Identifiers consist of letters, digits, -, ., _, starting with a >>> letter. >>> >>> $ qemu-system-x86_64 -qmp stdio -S >>> {"execute":"qmp_capabilities"} >>> {"return": {}} >>> {"execute":"netdev_add", "arguments": {"type": "user", "id": >>> "123/456"}} >>> {"return": {}} >>> >>> After: >>> >>> $ qemu-system-x86_64 -qmp stdio -S >>> {"execute":"qmp_capabilities"} >>> {"return": {}} >>> {"execute":"netdev_add", "arguments": {"type": "user", "id": >>> "123/456"}} >>> {"error": {"class": "GenericError", "desc": "Parameter "id" >>> expects an identifier"}} >>> >>> Validity checks should be performed always at the bottom of the call >>> chain, >>> because QMP skips all the steps above. Do this for the network >>> subsystem. >>> >>> Cc: Jason Wang <jasowang@redhat.com> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>> --- >>> net/net.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/net/net.c b/net/net.c >>> index 9c784da..d36729f 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -44,6 +44,7 @@ >>> #include "qemu/cutils.h" >>> #include "qemu/config-file.h" >>> #include "qemu/ctype.h" >>> +#include "qemu/id.h" >>> #include "qemu/iov.h" >>> #include "qemu/qemu-print.h" >>> #include "qemu/main-loop.h" >>> @@ -1011,6 +1012,17 @@ static int net_client_init1(const Netdev >>> *netdev, bool is_netdev, Error **errp) >>> } >>> } >>> + /* >>> + * The id for -net has already been checked by QemuOpts and >>> + * could be automatically generated, in which case it is not >>> + * well-formed by design. HMP and QMP only call us with >>> + * is_netdev == true. >>> + */ >>> + if (is_netdev && !id_wellformed(netdev->id)) { >>> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an >>> identifier"); >>> + return -1; >>> + } >>> + >>> nc = qemu_find_netdev(netdev->id); >>> if (nc) { >>> error_setg(errp, "Duplicate ID '%s'", netdev->id); >>> >> >> Sorry, I sent v2 yesterday. This patch passed the tests at the time >> it was submitted, but now fails (because it does not work with -nic). > > I don't see that. But I add a fixup in the pull request: > > https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg04237.html > > If it doesn't make sense, I will drop this and send a new pull request > next week. Hmm no, it didn't reach the list. Let me resend. Paolo
diff --git a/net/net.c b/net/net.c index 9c784da..d36729f 100644 --- a/net/net.c +++ b/net/net.c @@ -44,6 +44,7 @@ #include "qemu/cutils.h" #include "qemu/config-file.h" #include "qemu/ctype.h" +#include "qemu/id.h" #include "qemu/iov.h" #include "qemu/qemu-print.h" #include "qemu/main-loop.h" @@ -1011,6 +1012,17 @@ static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp) } } + /* + * The id for -net has already been checked by QemuOpts and + * could be automatically generated, in which case it is not + * well-formed by design. HMP and QMP only call us with + * is_netdev == true. + */ + if (is_netdev && !id_wellformed(netdev->id)) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); + return -1; + } + nc = qemu_find_netdev(netdev->id); if (nc) { error_setg(errp, "Duplicate ID '%s'", netdev->id);