From nobody Mon May 13 15:49:11 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1694163894; cv=none; d=zohomail.com; s=zohoarc; b=dTba4QQRKojnMErSQ9q6pluZumZYCTd/xXyf7cjvLVYkUpFfFow+pKkK1KOudiI9WNt3v8PGPPUuxNV3cXOXbybPGU9OphgZ2NaH7uWX21UakXt7Yg/OCj+LKb/KQadEz20VNJOzzva8zTgbUDkMLM9Osp+4idRkcMuXNySQzmY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1694163894; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=lqJSxQ0PRvxVS9xdJTx9whWQnMNtYY9aewoCtXHeV3Q=; b=Ow5dM5YKe/Xyxq/9kff4QazTnOifP0JjeUMROZT27d7z7OGNTULXmpW35L3aI26PcjvWe3PmKCx7S9ko56Gmrxw0cGFdqrYsGAyAFLFlG4XZDDPkKLqhwER5ueTd94W1QrpBOS5nfRmACcCZraEVT/owGcZYqhLWOMz1eP254l4= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1694163894230276.4483500218605; Fri, 8 Sep 2023 02:04:54 -0700 (PDT) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-624-A5Ku6rNIOPOnyA495NNNfQ-1; Fri, 08 Sep 2023 05:04:51 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A0ECD8030A9; Fri, 8 Sep 2023 09:04:48 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 73B1C40C6CCC; Fri, 8 Sep 2023 09:04:45 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id D3D1D194658F; Fri, 8 Sep 2023 09:04:44 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 30FAD194658C for ; Fri, 8 Sep 2023 09:04:44 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id F0E5640C84A5; Fri, 8 Sep 2023 09:04:43 +0000 (UTC) Received: from maggie.brq.redhat.com (unknown [10.43.2.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7C87440C2070 for ; Fri, 8 Sep 2023 09:04:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694163893; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=lqJSxQ0PRvxVS9xdJTx9whWQnMNtYY9aewoCtXHeV3Q=; b=aOhcFVgjFL+3kRQg6rENVrE7nQjY7LVEffSc8QQ53Bp8HpILDK0Bnynj5Fn9G8VghnylWC kbJTZs4w8zYILCjPQv+HyA5cPfE1rxl2sS3IO804p4ge9coxzRgLWAZJYlwpHaatst1Yxw rIG2K9ldkeHGIgXS0TNgiIMKBEo9pRY= X-MC-Unique: A5Ku6rNIOPOnyA495NNNfQ-1 X-Original-To: libvir-list@listman.corp.redhat.com From: Michal Privoznik To: libvir-list@redhat.com Subject: [PATCH] virnetdevopenvswitch: Propagate OVS error messages Date: Fri, 8 Sep 2023 11:04:38 +0200 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libvir-list-bounces@redhat.com Sender: "libvir-list" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1694163895698100001 Content-Type: text/plain; charset="utf-8"; x-default="true" When configuring OVS interfaces/bridges we spawn 'ovs-vsctl' with appropriate arguments and if it exited with a non-zero status we report a generic error message, like "Unable to add port vnet0 to OVS bridge ovsbr0". This is all cool, but the real reason why operation failed is hidden in (debug) logs because that's where virCommandRun() reports it unless caller requested otherwise. This is a bit clumsy because then we have to ask users to turn on debug logs and reproduce the problem again, e.g. [1]. Therefore, in cases where an error is reported to the user - just read ovs-vsctl's stderr and include it in the error message. For other cases (like VIR_DEBUG/VIR_WARN) - well they are meant to end up in (debug) logs anyway. 1: https://mail.openvswitch.org/pipermail/ovs-discuss/2023-September/052640= .html Signed-off-by: Michal Privoznik Reviewed-by: Martin Kletzander --- src/util/virnetdevopenvswitch.c | 93 ++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 35 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitc= h.c index 8dad6ed2bd..d836d05845 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -54,10 +54,14 @@ virNetDevOpenvswitchSetTimeout(unsigned int timeout) } =20 static virCommand * -virNetDevOpenvswitchCreateCmd(void) +virNetDevOpenvswitchCreateCmd(char **errbuf) { virCommand *cmd =3D virCommandNew(OVS_VSCTL); + virCommandAddArgFormat(cmd, "--timeout=3D%u", virNetDevOpenvswitchTime= out); + if (errbuf) + virCommandSetErrorBuffer(cmd, errbuf); + return cmd; } =20 @@ -137,6 +141,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, con= st char *ifname, char ifuuidstr[VIR_UUID_STRING_BUFLEN]; char vmuuidstr[VIR_UUID_STRING_BUFLEN]; g_autoptr(virCommand) cmd =3D NULL; + g_autofree char *errbuf =3D NULL; g_autofree char *attachedmac_ex_id =3D NULL; g_autofree char *ifaceid_ex_id =3D NULL; g_autofree char *profile_ex_id =3D NULL; @@ -157,7 +162,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, con= st char *ifname, ovsport->profileID); } =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "--", "--may-exist", "add-port", brname, ifname, NULL); =20 @@ -185,8 +190,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, con= st char *ifname, =20 if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to add port %1$s to OVS bridge %2$s"), - ifname, brname); + _("Unable to add port %1$s to OVS bridge %2$s: %3$s= "), + ifname, brname, NULLSTR(errbuf)); return -1; } =20 @@ -203,13 +208,15 @@ int virNetDevOpenvswitchAddPort(const char *brname, c= onst char *ifname, */ int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const= char *ifname) { - g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf =3D NULL; + g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); =20 virCommandAddArgList(cmd, "--", "--if-exists", "del-port", ifname, NUL= L); =20 if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to delete port %1$s from OVS"), ifname); + _("Unable to delete port %1$s from OVS: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 @@ -228,7 +235,8 @@ int virNetDevOpenvswitchRemovePort(const char *brname G= _GNUC_UNUSED, const char int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) { size_t len; - g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf =3D NULL; + g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); =20 virCommandAddArgList(cmd, "--if-exists", "get", "Interface", ifname, "external_ids:PortData", NULL); @@ -238,8 +246,8 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, = const char *ifname) /* Run the command */ if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to run command to get OVS port data for i= nterface %1$s"), - ifname); + _("Unable to run command to get OVS port data for i= nterface %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 @@ -263,21 +271,22 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate= , const char *ifname) int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) { g_autoptr(virCommand) cmd =3D NULL; + g_autofree char *errbuf =3D NULL; =20 if (!migrate) { VIR_DEBUG("No OVS port data for interface %s", ifname); return 0; } =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); virCommandAddArgFormat(cmd, "external_ids:PortData=3D%s", migrate); =20 /* Run the command */ if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to run command to set OVS port data for i= nterface %1$s"), - ifname); + _("Unable to run command to set OVS port data for i= nterface %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 @@ -371,7 +380,8 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) { - g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf =3D NULL; + g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); g_autofree char *output =3D NULL; =20 virCommandAddArgList(cmd, "--if-exists", "--format=3Dlist", "--data=3D= json", @@ -390,8 +400,9 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, if (virCommandRun(cmd, NULL) < 0 || STREQ_NULLABLE(output, "")) { /* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Interface not found")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Interface not found: %1$s"), + NULLSTR(errbuf)); return -1; } =20 @@ -433,7 +444,8 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) { - g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf =3D NULL; + g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); int exitstatus; =20 *master =3D NULL; @@ -443,8 +455,8 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifna= me, char **master) =20 if (virCommandRun(cmd, &exitstatus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to run command to get OVS master for inte= rface %1$s"), - ifname); + _("Unable to run command to get OVS master for inte= rface %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 @@ -546,7 +558,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, return 0; } =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(NULL); =20 if (server) { virCommandAddArgList(cmd, "--no-headings", "--columns=3Dname", "fi= nd", @@ -600,7 +612,8 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, int virNetDevOpenvswitchUpdateVlan(const char *ifname, const virNetDevVlan *virtVlan) { - g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf =3D NULL; + g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); =20 virCommandAddArgList(cmd, "--", "--if-exists", "clear", "Port", ifname, "ta= g", @@ -614,7 +627,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname, =20 if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set vlan configuration on port %1$s"),= ifname); + _("Unable to set vlan configuration on port %1$s: %= 2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 @@ -629,7 +643,7 @@ virNetDevOpenvswitchFindUUID(const char *table, g_autoptr(virCommand) cmd =3D NULL; char *uuid =3D NULL; =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(cmd, "--no-heading", "--columns=3D_uuid", "find",= table, vmid_ex_id, ifname_ex_id, NULL); virCommandSetOutputBuffer(cmd, &uuid); @@ -673,7 +687,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifn= ame, if (!*line) { continue; } - listcmd =3D virNetDevOpenvswitchCreateCmd(); + listcmd =3D virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(listcmd, "--no-heading", "--columns=3D_uu= id", "--if-exists", "list", "port", ifname, "qos", NULL); virCommandSetOutputBuffer(listcmd, &port_qos); @@ -681,13 +695,13 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *i= fname, VIR_WARN("Unable to remove port qos on port %s", ifname); } if (port_qos && *port_qos) { - g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCm= d(); + g_autoptr(virCommand) cmd =3D virNetDevOpenvswitchCreateCm= d(NULL); virCommandAddArgList(cmd, "remove", "port", ifname, "qos",= line, NULL); if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Unable to remove port qos on port %s", ifnam= e); } } - destroycmd =3D virNetDevOpenvswitchCreateCmd(); + destroycmd =3D virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(destroycmd, "destroy", "qos", line, NULL); if (virCommandRun(destroycmd, NULL) < 0) { VIR_WARN("Unable to destroy qos on port %s", ifname); @@ -706,7 +720,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifn= ame, continue; } =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(cmd, "destroy", "queue", line, NULL); if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Unable to destroy queue on port %s", ifname); @@ -722,15 +736,17 @@ static int virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) { g_autoptr(virCommand) cmd =3D NULL; + g_autofree char *errbuf =3D NULL; =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); virCommandAddArgFormat(cmd, "ingress_policing_rate=3D%llu", 0llu); virCommandAddArgFormat(cmd, "ingress_policing_burst=3D%llu", 0llu); =20 if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to reset ingress on port %1$s"), ifname); + _("Unable to reset ingress on port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 @@ -754,6 +770,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifnam= e, { char vmuuidstr[VIR_UUID_STRING_BUFLEN]; g_autoptr(virCommand) cmd =3D NULL; + g_autofree char *errbuf =3D NULL; g_autofree char *vmid_ex_id =3D NULL; g_autofree char *ifname_ex_id =3D NULL; g_autofree char *average =3D NULL; @@ -777,7 +794,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifnam= e, qos_uuid =3D virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex= _id); =20 /* create qos and set */ - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); if (queue_uuid && *queue_uuid) { g_auto(GStrv) lines =3D g_strsplit(queue_uuid, "\n", 0); virCommandAddArgList(cmd, "set", "queue", lines[0], NULL); @@ -806,17 +823,20 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifn= ame, if (virCommandRun(cmd, NULL) < 0) { if (queue_uuid && *queue_uuid) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set queue configuration on port %1= $s"), ifname); + _("Unable to set queue configuration on port %1= $s: %2$s"), + ifname, NULLSTR(errbuf)); } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to create and set qos configuration o= n port %1$s"), ifname); + _("Unable to create and set qos configuration o= n port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); } return -1; } =20 if (qos_uuid && *qos_uuid) { g_auto(GStrv) lines =3D g_strsplit(qos_uuid, "\n", 0); - g_autoptr(virCommand) qoscmd =3D virNetDevOpenvswitchCreateCmd(); + g_autofree char *qoserrbuf =3D NULL; + g_autoptr(virCommand) qoscmd =3D virNetDevOpenvswitchCreateCmd(&qo= serrbuf); =20 virCommandAddArgList(qoscmd, "set", "qos", lines[0], NULL); virCommandAddArgFormat(qoscmd, "other_config:min-rate=3D%s", avera= ge); @@ -829,7 +849,8 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifnam= e, virCommandAddArgList(qoscmd, vmid_ex_id, ifname_ex_id, NULL); if (virCommandRun(qoscmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set qos configuration on port %1$s= "), ifname); + _("Unable to set qos configuration on port %1$s= : %2$s"), + ifname, NULLSTR(qoserrbuf)); return -1; } } @@ -842,8 +863,9 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifnam= e, const virNetDevBandwidthRate *rx) { g_autoptr(virCommand) cmd =3D NULL; + g_autofree char *errbuf =3D NULL; =20 - cmd =3D virNetDevOpenvswitchCreateCmd(); + cmd =3D virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); virCommandAddArgFormat(cmd, "ingress_policing_rate=3D%llu", rx->average * VIR_NETDEV_RX_TO_OVS); @@ -853,7 +875,8 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifnam= e, =20 if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set vlan configuration on port %1$s"),= ifname); + _("Unable to set vlan configuration on port %1$s: %= 2$s"), + ifname, NULLSTR(errbuf)); return -1; } =20 --=20 2.41.0