From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gD6CW-00052h-O4 for qemu-devel@nongnu.org; Thu, 18 Oct 2018 07:10:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gD6CT-0002yK-Kn for qemu-devel@nongnu.org; Thu, 18 Oct 2018 07:10:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39768) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gD6CT-0002xM-9F for qemu-devel@nongnu.org; Thu, 18 Oct 2018 07:10:45 -0400 Date: Thu, 18 Oct 2018 13:10:41 +0200 From: Kevin Wolf Message-ID: <20181018111041.GA4946@localhost.localdomain> References: <20181017082702.5581-1-armbru@redhat.com> <20181017082702.5581-4-armbru@redhat.com> <20181017135043.GE31479@localhost.localdomain> <878t2wo518.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <878t2wo518.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v4 03/38] block: Use warn_report() & friends to report warnings List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Peter Lieven , qemu-devel@nongnu.org, Ronnie Sahlberg , Liu Yuan , Paolo Bonzini Am 17.10.2018 um 19:29 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 17.10.2018 um 10:26 hat Markus Armbruster geschrieben: > >> Calling error_report() in a function that takes an Error ** argument > >> is suspicious. Convert a few that are actually warnings to > >> warn_report(). > >> > >> While there, split warnings consisting of multiple sentences to > >> conform to conventions spelled out in warn_report()'s contract, and > >> improve a rather useless warning in sheepdog.c. > >> > >> Cc: Kevin Wolf > >> Cc: Ronnie Sahlberg > >> Cc: Paolo Bonzini > >> Cc: Peter Lieven > >> Cc: Hitoshi Mitake > >> Cc: Liu Yuan > >> Signed-off-by: Markus Armbruster > >> Reviewed-by: Eric Blake > >> --- > >> block/bochs.c | 8 ++++---- > >> block/cloop.c | 8 ++++---- > >> block/dmg.c | 8 ++++---- > >> block/iscsi.c | 2 +- > >> block/rbd.c | 12 ++++++------ > >> block/sheepdog.c | 2 +- > >> block/vvfat.c | 8 ++++---- > >> 7 files changed, 24 insertions(+), 24 deletions(-) > >> > >> diff --git a/block/bochs.c b/block/bochs.c > >> index 50c630047b..36c1b45bd2 100644 > >> --- a/block/bochs.c > >> +++ b/block/bochs.c > >> @@ -112,10 +112,10 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, > >> } > >> > >> if (!bdrv_is_read_only(bs)) { > >> - error_report("Opening bochs images without an explicit read-only=on " > >> - "option is deprecated. Future versions will refuse to " > >> - "open the image instead of automatically marking the " > >> - "image read-only."); > >> + warn_report("Opening bochs images without an explicit read-only=on " > >> + "option is deprecated"); > >> + error_printf("Future versions may refuse to open the image " > >> + "instead of automatically marking it read-only.\n"); > >> ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */ > >> if (ret < 0) { > >> return ret; > > > > While I agree with your intention, it would be best to leave all of the > > !bdrv_is_read_only() warnings alone. My series "[PATCH v2 0/8] block: > > Add auto-read-only option" gets rid of the message entirely, so this > > would only add a merge conflict. > > I wasn't are of that series. I'm going to drop these hunks. Left: > > diff --git a/block/iscsi.c b/block/iscsi.c > index bb69faf34a..73998c2860 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1844,7 +1844,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > iscsi_set_timeout(iscsi, timeout); > #else > if (timeout) { > - error_report("iSCSI: ignoring timeout value for libiscsi <1.15.0"); > + warn_report("iSCSI: ignoring timeout value for libiscsi <1.15.0"); > } > #endif > > diff --git a/block/rbd.c b/block/rbd.c > index 014c68d629..6e26bac170 100644 > --- a/block/rbd.c > +++ b/block/rbd.c > @@ -750,8 +750,8 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, > /* Take care whenever deciding to actually deprecate; once this ability > * is removed, we will not be able to open any images with legacy-styled > * backing image strings. */ > - error_report("RBD options encoded in the filename as keyvalue pairs " > - "is deprecated"); > + warn_report("RBD options encoded in the filename as keyvalue pairs " > + "is deprecated"); > } > > /* Remove the processed options from the QDict (the visitor processes > diff --git a/block/sheepdog.c b/block/sheepdog.c > index b229a664d9..0125df9d49 100644 > --- a/block/sheepdog.c > +++ b/block/sheepdog.c > @@ -572,7 +572,7 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error **errp) > if (s->addr->type == SOCKET_ADDRESS_TYPE_INET && fd >= 0) { > int ret = socket_set_nodelay(fd); > if (ret < 0) { > - error_report("%s", strerror(errno)); > + warn_report("can't set TCP_NODELAY: %s", strerror(errno)); > } > } Looks good to me. Reviewed-by: Kevin Wolf