From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EFDFAC56201 for ; Tue, 24 Nov 2020 14:05:22 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0E690206FB for ; Tue, 24 Nov 2020 14:05:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=daynix-com.20150623.gappssmtp.com header.i=@daynix-com.20150623.gappssmtp.com header.b="vkTfaIRH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0E690206FB Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=daynix.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:48190 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1khYwa-0006dG-Ob for qemu-devel@archiver.kernel.org; Tue, 24 Nov 2020 09:05:20 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:36436) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1khYvE-0006B6-UL for qemu-devel@nongnu.org; Tue, 24 Nov 2020 09:03:56 -0500 Received: from mail-ot1-x343.google.com ([2607:f8b0:4864:20::343]:40983) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1khYvC-00057D-Lu for qemu-devel@nongnu.org; Tue, 24 Nov 2020 09:03:56 -0500 Received: by mail-ot1-x343.google.com with SMTP id o3so19408624ota.8 for ; Tue, 24 Nov 2020 06:03:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=daynix-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=aJ4lfhx8djW0Z0WvrYqOJN5dSR/azb3OIQKXQjFH3lY=; b=vkTfaIRHjjtkHbW7IC6DPoCNW0Rcxstzi7gz57BYs9p6L0YhTKBdQO/H67csIpn0Sp MHMACTTW4MygJTRP4weAXPZtofwt7oztGH8myiHb+AKewdN44MbEniN0vtrRcb6gz0gl RQt2dX0+bmlA8polnKJ+A1THucXgLiXznkJChN13wqMIfm+fR2gqA/VkKa2T4LiER381 57RH6XBJqHPhwX1GjYJx41MTGxTohIelHj0iwW+2vU5wuJoG8FTYwgBflLm3hxTZKly6 VP4mTkCsYbmgBoQeBtaz8ZCjOCmZ0hXJoqv3E19UPxZGj7p0MvK8kfzx3u2E8PkSkRLc jHow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=aJ4lfhx8djW0Z0WvrYqOJN5dSR/azb3OIQKXQjFH3lY=; b=I5IIagk0g8MSJde5FSU3yYZAb5eQfU64ZPhQA9CHpozDSU74Kvr8VUN0O1YnXbTgjA AxH9ZDmNTiH8EOWwDbwDwTj8jQ1QUUPohw432NjcSmLh4aIC19Uvm2t5QslsAp2Nm+Xx 4thjR1G58iVSBiX2zz7DuXmQxN+roFgtn+C3a1vG5CXdjd55+MJqzs1Ig8qD3vwF+7Rx 6BOlI+PqwSFU7elbUBJqv8S/zlLvcDmP93ot3Hc7tBJY1Fx6lImbwy/x/HiOCqeEIAWO 1uujD8rRntUKJJ32giv393aHUhhgmCtUg+y+Css2pH4ixUlEw7AzdjZWs8P0eV6Dv+pV RXzg== X-Gm-Message-State: AOAM5334VLNEKl14WxNttlOsaOvDvujOrXuT83GvOYjk1chWXP9e3yB1 LatyGE5Se8FT9JrMrtzc82HBrKrQ7DzdT3OCnGSIdQ== X-Google-Smtp-Source: ABdhPJxMOGxuPU6sXrRDVKGB4W9qzD30hKNb6cMrTZMhGrCoEgWLMwdQ6Z/pOipiS+CVzTYTBmjyaaS7YGsMj+gIezI= X-Received: by 2002:a9d:4715:: with SMTP id a21mr3429552otf.220.1606226633408; Tue, 24 Nov 2020 06:03:53 -0800 (PST) MIME-Version: 1.0 References: <20200716035532.1407660-1-andrew@daynix.com> <874klk5gnc.fsf@dusky.pond.sub.org> <87lfesv2zu.fsf@dusky.pond.sub.org> <87blfnp20k.fsf@dusky.pond.sub.org> <87lferm4x5.fsf@dusky.pond.sub.org> <87tutej3dc.fsf@dusky.pond.sub.org> <87blfmj2qx.fsf@dusky.pond.sub.org> In-Reply-To: <87blfmj2qx.fsf@dusky.pond.sub.org> From: Yuri Benditovich Date: Tue, 24 Nov 2020 16:03:42 +0200 Message-ID: Subject: Re: [PATCH] hmp: Changed hmp_netdev_add() using qmp_marshal_netdev_add() To: Markus Armbruster Content-Type: multipart/alternative; boundary="000000000000e9dd6d05b4dac783" Received-SPF: none client-ip=2607:f8b0:4864:20::343; envelope-from=yuri.benditovich@daynix.com; helo=mail-ot1-x343.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Yan Vugenfirer , Andrew Melnichenko , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000e9dd6d05b4dac783 Content-Type: text/plain; charset="UTF-8" On Tue, Nov 24, 2020 at 3:36 PM Markus Armbruster wrote: > Markus Armbruster writes: > > > Yuri Benditovich writes: > > > >> Please confirm that this patch is intended to solve only the problem > with > >> hmp (and disallow duplicated ids) > > > > The intent is to reject duplicate ID and to accept non-duplicate ID, no > > matter how the device is created (CLI, HMP, QMP) or a prior instance was > > deleted (HMP, QMP). > > > >> With it the netdev that was added from qemu's command line and was > deleted > >> (for example by hmp) still can't be created, correct? > > > > Yet another case; back to the drawing board... > > Next try. Hope this is one holds water :) > > > diff --git a/net/net.c b/net/net.c > index 794c652282..c1dc75fc37 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -978,6 +978,7 @@ static int (* const > net_client_init_fun[NET_CLIENT_DRIVER__MAX])( > static int net_client_init1(const Netdev *netdev, bool is_netdev, Error > **errp) > { > NetClientState *peer = NULL; > + NetClientState *nc; > > if (is_netdev) { > if (netdev->type == NET_CLIENT_DRIVER_NIC || > @@ -1005,6 +1006,12 @@ static int net_client_init1(const Netdev *netdev, > bool is_netdev, Error **errp) > } > } > > + nc = qemu_find_netdev(netdev->id); > + if (nc) { > + error_setg(errp, "Duplicate ID '%s'", netdev->id); > + return -1; > + } > + > if (net_client_init_fun[netdev->type](netdev, netdev->id, peer, errp) > < 0) { > /* FIXME drop when all init functions store an Error */ > if (errp && !*errp) { > @@ -1015,8 +1022,6 @@ static int net_client_init1(const Netdev *netdev, > bool is_netdev, Error **errp) > } > > if (is_netdev) { > - NetClientState *nc; > - > nc = qemu_find_netdev(netdev->id); > assert(nc); > nc->is_netdev = true; > @@ -1137,6 +1142,7 @@ void qmp_netdev_add(Netdev *netdev, Error **errp) > void qmp_netdev_del(const char *id, Error **errp) > { > NetClientState *nc; > + QemuOpts *opts; > > nc = qemu_find_netdev(id); > if (!nc) { > @@ -1151,6 +1157,16 @@ void qmp_netdev_del(const char *id, Error **errp) > } > > qemu_del_net_client(nc); > + > + /* > + * Wart: we need to delete the QemuOpts associated with netdevs > + * created via CLI or HMP, to avoid bogus "Duplicate ID" errors in > + * HMP netdev_add. > + */ > + opts = qemu_opts_find(qemu_find_opts("netdev"), id); > + if (opts) { > + qemu_opts_del(opts); > + } > } > > With this part there is no need to unconditionally delete the options in hmp_netdev_add, correct? > static void netfilter_print_info(Monitor *mon, NetFilterState *nf) > -- > 2.26.2 > > --000000000000e9dd6d05b4dac783 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Nov 24, 2020= at 3:36 PM Markus Armbruster <armb= ru@redhat.com> wrote:
Markus Armbruster <armbru@redhat.com> writes:

> Yuri Benditovich <yuri.benditovich@daynix.com> writes:
>
>> Please confirm that this patch is intended to solve only the probl= em with
>> hmp (and disallow duplicated ids)
>
> The intent is to reject duplicate ID and to accept non-duplicate ID, n= o
> matter how the device is created (CLI, HMP, QMP) or a prior instance w= as
> deleted (HMP, QMP).
>
>> With it the netdev that was added from qemu's command line and= was deleted
>> (for example by hmp) still can't be created, correct?
>
> Yet another case; back to the drawing board...

Next try.=C2=A0 Hope this is one holds water :)


diff --git a/net/net.c b/net/net.c
index 794c652282..c1dc75fc37 100644
--- a/net/net.c
+++ b/net/net.c
@@ -978,6 +978,7 @@ static int (* const net_client_init_fun[NET_CLIENT_DRIV= ER__MAX])(
=C2=A0static int net_client_init1(const Netdev *netdev, bool is_netdev, Err= or **errp)
=C2=A0{
=C2=A0 =C2=A0 =C2=A0NetClientState *peer =3D NULL;
+=C2=A0 =C2=A0 NetClientState *nc;

=C2=A0 =C2=A0 =C2=A0if (is_netdev) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (netdev->type =3D=3D NET_CLIENT_DRI= VER_NIC ||
@@ -1005,6 +1006,12 @@ static int net_client_init1(const Netdev *netdev, bo= ol is_netdev, Error **errp)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0}

+=C2=A0 =C2=A0 nc =3D qemu_find_netdev(netdev->id);
+=C2=A0 =C2=A0 if (nc) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 error_setg(errp, "Duplicate ID '%s= 9;", netdev->id);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
+=C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0if (net_client_init_fun[netdev->type](netdev, netdev= ->id, peer, errp) < 0) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* FIXME drop when all init functions sto= re an Error */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (errp && !*errp) {
@@ -1015,8 +1022,6 @@ static int net_client_init1(const Netdev *netdev, boo= l is_netdev, Error **errp)
=C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0if (is_netdev) {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 NetClientState *nc;
-
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0nc =3D qemu_find_netdev(netdev->id); =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0assert(nc);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0nc->is_netdev =3D true;
@@ -1137,6 +1142,7 @@ void qmp_netdev_add(Netdev *netdev, Error **errp)
=C2=A0void qmp_netdev_del(const char *id, Error **errp)
=C2=A0{
=C2=A0 =C2=A0 =C2=A0NetClientState *nc;
+=C2=A0 =C2=A0 QemuOpts *opts;

=C2=A0 =C2=A0 =C2=A0nc =3D qemu_find_netdev(id);
=C2=A0 =C2=A0 =C2=A0if (!nc) {
@@ -1151,6 +1157,16 @@ void qmp_netdev_del(const char *id, Error **errp) =C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0qemu_del_net_client(nc);
+
+=C2=A0 =C2=A0 /*
+=C2=A0 =C2=A0 =C2=A0* Wart: we need to delete the QemuOpts associated with= netdevs
+=C2=A0 =C2=A0 =C2=A0* created via CLI or HMP, to avoid bogus "Duplica= te ID" errors in
+=C2=A0 =C2=A0 =C2=A0* HMP netdev_add.
+=C2=A0 =C2=A0 =C2=A0*/
+=C2=A0 =C2=A0 opts =3D qemu_opts_find(qemu_find_opts("netdev"), = id);
+=C2=A0 =C2=A0 if (opts) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_opts_del(opts);
+=C2=A0 =C2=A0 }
=C2=A0}


With this part there is no need to unc= onditionally delete the options in=C2=A0= hmp_netdev_add, correct?
=C2=A0
=C2=A0static void netfilter_print_info(Monitor *mon, NetFilterState *nf) --
2.26.2

--000000000000e9dd6d05b4dac783--