[PATCH 0/2] Prohibit newlines in VIR_WARN strings
Make them the same as we do with error messages since they often end up in logs. Since they are not translated [1] the existing check didn't catch that. [1] IMO in some cases they are in fact used as errors, and maybe would be worth to be translated. But this is for another series Peter Krempa (2): Don't break up strings for VIR_WARN messages syntax-check: Enforce no linebreaks in VIR_WARN messages build-aux/syntax-check.mk | 8 +++++ src/bhyve/bhyve_command.c | 4 +-- src/conf/virdomainjob.c | 5 +-- src/cpu/cpu_x86.c | 5 ++- src/esx/esx_vi.c | 3 +- src/hypervisor/virhostdev.c | 12 +++---- src/interface/interface_backend_netcf.c | 12 +++---- src/libxl/libxl_conf.c | 4 +-- src/libxl/xen_xl.c | 4 +-- src/lxc/lxc_driver.c | 3 +- src/lxc/lxc_process.c | 3 +- src/network/bridge_driver.c | 15 +++------ src/nwfilter/nwfilter_dhcpsnoop.c | 3 +- src/qemu/qemu_block.c | 3 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_conf.c | 4 +-- src/qemu/qemu_domain.c | 44 +++++++++---------------- src/qemu/qemu_domain_address.c | 10 ++---- src/qemu/qemu_driver.c | 10 +++--- src/qemu/qemu_firmware.c | 6 ++-- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_migration.c | 11 +++---- src/qemu/qemu_monitor_json.c | 4 +-- src/remote/remote_driver.c | 3 +- src/rpc/virnetserverclient.c | 5 ++- src/security/security_dac.c | 3 +- src/security/security_selinux.c | 7 ++-- src/security/security_stack.c | 34 +++++++------------ src/storage/storage_backend_rbd.c | 4 +-- src/storage/storage_backend_zfs.c | 3 +- src/storage/storage_util.c | 4 +-- src/storage_file/storage_file_probe.c | 3 +- src/vbox/vbox_common.c | 4 +-- src/vmx/vmx.c | 6 ++-- src/vz/vz_sdk.c | 3 +- 35 files changed, 96 insertions(+), 162 deletions(-) -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> The 'warn' level messages are logged in the default settings so may end up in something which the user looks for. Random line breaks prevent grepping for the message. Similarly to 'error' level messages remove the arbitrary line breaks in the source to make the messages greppable in the source. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_command.c | 4 +-- src/conf/virdomainjob.c | 5 +-- src/cpu/cpu_x86.c | 5 ++- src/esx/esx_vi.c | 3 +- src/hypervisor/virhostdev.c | 12 +++---- src/interface/interface_backend_netcf.c | 12 +++---- src/libxl/libxl_conf.c | 4 +-- src/libxl/xen_xl.c | 4 +-- src/lxc/lxc_driver.c | 3 +- src/lxc/lxc_process.c | 3 +- src/network/bridge_driver.c | 15 +++------ src/nwfilter/nwfilter_dhcpsnoop.c | 3 +- src/qemu/qemu_block.c | 3 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_conf.c | 4 +-- src/qemu/qemu_domain.c | 44 +++++++++---------------- src/qemu/qemu_domain_address.c | 10 ++---- src/qemu/qemu_driver.c | 10 +++--- src/qemu/qemu_firmware.c | 6 ++-- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_migration.c | 11 +++---- src/qemu/qemu_monitor_json.c | 4 +-- src/remote/remote_driver.c | 3 +- src/rpc/virnetserverclient.c | 5 ++- src/security/security_dac.c | 3 +- src/security/security_selinux.c | 7 ++-- src/security/security_stack.c | 34 +++++++------------ src/storage/storage_backend_rbd.c | 4 +-- src/storage/storage_backend_zfs.c | 3 +- src/storage/storage_util.c | 4 +-- src/storage_file/storage_file_probe.c | 3 +- src/vbox/vbox_common.c | 4 +-- src/vmx/vmx.c | 6 ++-- src/vz/vz_sdk.c | 3 +- 34 files changed, 88 insertions(+), 162 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 8671644cc8..1da344f503 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -1154,9 +1154,7 @@ virBhyveProcessBuildBhyveCmd(struct _bhyveConn *driver, virDomainDef *def, if (def->namespaceData) { bhyveDomainCmdlineDef *bhyvecmd; - VIR_WARN("Booting the guest using command line pass-through feature, " - "which could potentially cause inconsistent state and " - "upgrade issues"); + VIR_WARN("Booting the guest using command line pass-through feature, which could potentially cause inconsistent state and upgrade issues"); bhyvecmd = def->namespaceData; for (i = 0; i < bhyvecmd->num_args; i++) diff --git a/src/conf/virdomainjob.c b/src/conf/virdomainjob.c index c2e7d33097..41d7b1b742 100644 --- a/src/conf/virdomainjob.c +++ b/src/conf/virdomainjob.c @@ -432,10 +432,7 @@ virDomainObjBeginJobInternal(virDomainObj *obj, if (jobObj->asyncJob && jobObj->asyncStarted) asyncDuration = now - jobObj->asyncStarted; - VIR_WARN("Cannot start job (%s, %s, %s) in API %s for domain %s; " - "current job is (%s, %s, %s) " - "owned by (%llu %s, %llu %s, %llu %s (flags=0x%x)) " - "for (%llus, %llus, %llus)", + VIR_WARN("Cannot start job (%s, %s, %s) in API %s for domain %s; current job is (%s, %s, %s) owned by (%llu %s, %llu %s, %llu %s (flags=0x%x)) for (%llus, %llus, %llus)", virDomainJobTypeToString(job), virDomainAgentJobTypeToString(agentJob), virDomainAsyncJobTypeToString(asyncJob), diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 0f7eb8f48b..3a18d859dc 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2326,9 +2326,8 @@ x86Decode(virCPUDef *cpu, preferred[0]); return -1; } else { - VIR_WARN("Preferred CPU model %s not allowed by" - " hypervisor; closest supported model will be" - " used", preferred[0]); + VIR_WARN("Preferred CPU model %s not allowed by hypervisor; closest supported model will be used", + preferred[0]); } } else { VIR_DEBUG("CPU model %s not allowed by hypervisor; ignoring", diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index cfd783622d..db006ed16f 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2478,8 +2478,7 @@ esxVI_GetVirtualMachineIdentity(esxVI_ObjectContent *virtualMachine, } else { memset(uuid, 0, VIR_UUID_BUFLEN); - VIR_WARN("Cannot access UUID, because 'configStatus' property " - "indicates a config problem"); + VIR_WARN("Cannot access UUID, because 'configStatus' property indicates a config problem"); } } diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index b063a44f62..720ed9bb60 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -1839,8 +1839,7 @@ virHostdevReAttachUSBDevices(virHostdevManager *mgr, tmp = virUSBDeviceListFind(mgr->activeUSBHostdevs, usb); if (!tmp) { - VIR_WARN("Unable to find device %03d.%03d " - "in list of active USB devices", + VIR_WARN("Unable to find device %03d.%03d in list of active USB devices", usbsrc->bus, usbsrc->device); continue; } @@ -1881,8 +1880,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManager *mgr, * because qemuProcessStart could fail half way through. */ if (!(tmp = virSCSIDeviceListFind(mgr->activeSCSIHostdevs, scsi))) { - VIR_WARN("Unable to find device %s:%u:%u:%llu " - "in list of active SCSI devices", + VIR_WARN("Unable to find device %s:%u:%u:%llu in list of active SCSI devices", scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit); return; @@ -1965,8 +1963,7 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManager *mgr, if (!(tmp = virSCSIVHostDeviceListFind(mgr->activeSCSIVHostHostdevs, host))) { - VIR_WARN("Unable to find device %s " - "in list of active SCSI_host devices", + VIR_WARN("Unable to find device %s in list of active SCSI_host devices", hostsrc->wwpn); virObjectUnlock(mgr->activeSCSIVHostHostdevs); return; @@ -2311,8 +2308,7 @@ virHostdevPrepareOneNVMeDevice(virHostdevManager *hostdev_mgr, if (drvType == VIR_PCI_STUB_DRIVER_VFIO || STREQ_NULLABLE(drvName, "nvme")) continue; - VIR_WARN("Suspicious NVMe disk assignment. PCI device " - "%s is not an NVMe disk, it has %s driver", + VIR_WARN("Suspicious NVMe disk assignment. PCI device %s is not an NVMe disk, it has %s driver", virPCIDeviceGetName(pci), NULLSTR(drvName)); } diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index 16e1215663..8ab2313668 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -396,8 +396,8 @@ static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn, /* Ignore the NETCF_NOERROR, as the interface is very likely * deleted by other management apps (e.g. virt-manager). */ - VIR_WARN("couldn't find interface named '%s', might be " - "deleted by other process", names[i]); + VIR_WARN("couldn't find interface named '%s', might be deleted by other process", + names[i]); continue; } } @@ -487,8 +487,8 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn, /* Ignore the NETCF_NOERROR, as the interface is very likely * deleted by other management apps (e.g. virt-manager). */ - VIR_WARN("couldn't find interface named '%s', might be " - "deleted by other process", allnames[i]); + VIR_WARN("couldn't find interface named '%s', might be deleted by other process", + allnames[i]); continue; } } @@ -669,8 +669,8 @@ netcfConnectListAllInterfaces(virConnectPtr conn, /* Ignore the NETCF_NOERROR, as the interface is very likely * deleted by other management apps (e.g. virt-manager). */ - VIR_WARN("couldn't find interface named '%s', might be " - "deleted by other process", names[i]); + VIR_WARN("couldn't find interface named '%s', might be deleted by other process", + names[i]); continue; } } diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2b988157fa..d25b92b4de 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -550,9 +550,7 @@ libxlMakeDomBuildInfo(virDomainDef *def, } if (def->cpu && def->cpu->mode == VIR_CPU_MODE_CUSTOM) { - VIR_WARN("Ignoring CPU with mode=custom, update your config to " - "mode=host-passthrough to avoid risk of changed guest " - "semantics when mode=custom is supported in the future"); + VIR_WARN("Ignoring CPU with mode=custom, update your config to mode=host-passthrough to avoid risk of changed guest semantics when mode=custom is supported in the future"); } } diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 42e1408cf3..d05972af5b 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1255,8 +1255,8 @@ xenFormatXLCPUID(virConf *conf, virDomainDef *def) return 0; if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { - VIR_WARN("ignoring CPU mode '%s', only host-passthrough mode " - "is supported", virCPUModeTypeToString(def->cpu->mode)); + VIR_WARN("ignoring CPU mode '%s', only host-passthrough mode is supported", + virCPUModeTypeToString(def->cpu->mode)); return 0; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3ec6d298bd..69eafc110d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3556,8 +3556,7 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver, if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0) goto cleanup; } else { - VIR_WARN("setting bandwidth on interfaces of " - "type '%s' is not implemented yet: %s", + VIR_WARN("setting bandwidth on interfaces of type '%s' is not implemented yet: %s", virDomainNetTypeToString(actualType), virGetLastErrorMessage()); } } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index e22f77ef83..2c0bcb9dd3 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -617,8 +617,7 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver, if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0) goto cleanup; } else { - VIR_WARN("setting bandwidth on interfaces of " - "type '%s' is not implemented yet", + VIR_WARN("setting bandwidth on interfaces of type '%s' is not implemented yet", virDomainNetTypeToString(type)); } } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ecfce5d9a4..4dc3e5424b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -714,8 +714,8 @@ networkStateInitialize(bool privileged, #ifdef WITH_FIREWALLD if (!virGDBusHasSystemBus() || !(sysbus = virGDBusGetSystemBus())) { - VIR_WARN("DBus not available, disabling firewalld support " - "in bridge_network_driver: %s", virGetLastErrorMessage()); + VIR_WARN("DBus not available, disabling firewalld support in bridge_network_driver: %s", + virGetLastErrorMessage()); } else { g_dbus_connection_signal_subscribe(sysbus, NULL, @@ -1389,12 +1389,7 @@ networkDnsmasqConfContents(virNetworkObj *obj, } if (ipv6def && ipv6SLAAC) { - VIR_WARN("For IPv6, when DHCP is specified for one address, then " - "state-full Router Advertising will occur. The additional " - "IPv6 addresses specified require manually configured guest " - "network to work properly since both state-full (DHCP) " - "and state-less (SLAAC) addressing are not supported " - "on the same network interface."); + VIR_WARN("For IPv6, when DHCP is specified for one address, then state-full Router Advertising will occur. The additional IPv6 addresses specified require manually configured guest network to work properly since both state-full (DHCP) and state-less (SLAAC) addressing are not supported on the same network interface."); } if (networkDnsmasqConfDHCP(&configbuf, ipv4def, def->bridge, &nbleases, dctx) < 0 || @@ -2291,9 +2286,7 @@ networkCreateInterfacePool(virNetworkDef *netdef) thisIf->type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; netdef->forward.nifs++; } else { - VIR_WARN("VF %zu of SRIOV PF %s couldn't be added to the " - "interface pool because it isn't bound " - "to a network driver - possibly in use elsewhere", + VIR_WARN("VF %zu of SRIOV PF %s couldn't be added to the interface pool because it isn't bound to a network driver - possibly in use elsewhere", i, netdef->forward.pfs->dev); } break; diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index e65dbb93d9..916c6fafa1 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1331,8 +1331,7 @@ virNWFilterDHCPSnoopThread(void *req0) pcapConf[i].maxQSize) { if (last_displayed_queue - time(0) > 10) { last_displayed_queue = time(0); - VIR_WARN("Worker thread for interface '%s' has a " - "job queue that is too long", + VIR_WARN("Worker thread for interface '%s' has a job queue that is too long", req->binding->portdevname); } continue; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7986db8e76..324c2635b4 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2869,8 +2869,7 @@ qemuBlockRemoveImageMetadata(virQEMUDriver *driver, for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) { - VIR_WARN("Unable to remove disk metadata on " - "vm %s from %s (disk target %s)", + VIR_WARN("Unable to remove disk metadata on vm %s from %s (disk target %s)", vm->def->name, NULLSTR(n->path), diskTarget); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5b7423e354..a4445ef17a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9128,8 +9128,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, goto cleanup; } } else { - VIR_WARN("setting bandwidth on interfaces of " - "type '%s' is not implemented yet", + VIR_WARN("setting bandwidth on interfaces of type '%s' is not implemented yet", virDomainNetTypeToString(actualType)); } } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f2f9cc8a21..9c32310096 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1071,9 +1071,7 @@ virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfig *cfg, return -1; if (fwList) { - VIR_WARN("Obsolete nvram variable is set while firmware metadata " - "files found. Note that the nvram config file variable is " - "going to be ignored."); + VIR_WARN("Obsolete nvram variable is set while firmware metadata files found. Note that the nvram config file variable is going to be ignored."); return 0; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 91d57ee60b..dde257bb70 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -291,8 +291,7 @@ qemuDomainObjPrivateXMLParseJobNBD(virDomainObj *vm, if (n > 0) { if (vm->job->asyncJob != VIR_ASYNC_JOB_MIGRATION_OUT) { - VIR_WARN("Found disks marked for migration but we were not " - "migrating"); + VIR_WARN("Found disks marked for migration but we were not migrating"); n = 0; } for (i = 0; i < n; i++) { @@ -5140,11 +5139,9 @@ qemuDomainObjEnterMonitorInternal(virDomainObj *obj, return -1; } } else if (obj->job->asyncOwner == virThreadSelfID()) { - VIR_WARN("This thread seems to be the async job owner; entering" - " monitor without asking for a nested job is dangerous"); + VIR_WARN("This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous"); } else if (obj->job->owner != virThreadSelfID()) { - VIR_WARN("Entering a monitor without owning a job. " - "Job %s owner %s (%llu)", + VIR_WARN("Entering a monitor without owning a job. Job %s owner %s (%llu)", virDomainJobTypeToString(obj->job->active), obj->job->ownerAPI, obj->job->owner); } @@ -11094,8 +11091,7 @@ syncNicRxFilterMacAddr(char *ifname, virNetDevRxFilter *guestFilter, /* set new MAC address from guest to associated macvtap device */ if (virNetDevSetMAC(ifname, &guestFilter->mac) < 0) { - VIR_WARN("Couldn't set new MAC address %s to device %s " - "while responding to NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't set new MAC address %s to device %s while responding to NIC_RX_FILTER_CHANGED", newMacStr, ifname); } else { VIR_DEBUG("device %s MAC address set to %s", ifname, newMacStr); @@ -11127,8 +11123,7 @@ syncNicRxFilterGuestMulticast(char *ifname, virNetDevRxFilter *guestFilter, virMacAddrFormat(&guestFilter->multicast.table[i], macstr); if (virNetDevAddMulti(ifname, &guestFilter->multicast.table[i]) < 0) { - VIR_WARN("Couldn't add new multicast MAC address %s to " - "device %s while responding to NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't add new multicast MAC address %s to device %s while responding to NIC_RX_FILTER_CHANGED", macstr, ifname); } else { VIR_DEBUG("Added multicast MAC %s to %s interface", @@ -11162,8 +11157,7 @@ syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilter *guestFilter, virMacAddrFormat(&hostFilter->multicast.table[i], macstr); if (virNetDevDelMulti(ifname, &hostFilter->multicast.table[i]) < 0) { - VIR_WARN("Couldn't delete multicast MAC address %s from " - "device %s while responding to NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't delete multicast MAC address %s from device %s while responding to NIC_RX_FILTER_CHANGED", macstr, ifname); } else { VIR_DEBUG("Deleted multicast MAC %s from %s interface", @@ -11196,8 +11190,7 @@ syncNicRxFilterPromiscMode(char *ifname, if (setpromisc) { if (virNetDevSetPromiscuous(ifname, promisc) < 0) { - VIR_WARN("Couldn't set PROMISC flag to %s for device %s " - "while responding to NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't set PROMISC flag to %s for device %s while responding to NIC_RX_FILTER_CHANGED", promisc ? "true" : "false", ifname); } } @@ -11214,9 +11207,8 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilter *guestFilter, switch (guestFilter->multicast.mode) { case VIR_NETDEV_RX_FILTER_MODE_ALL: if (virNetDevSetRcvAllMulti(ifname, true) < 0) { - VIR_WARN("Couldn't set allmulticast flag to 'on' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", ifname); + VIR_WARN("Couldn't set allmulticast flag to 'on' for device %s while responding to NIC_RX_FILTER_CHANGED", + ifname); } break; @@ -11227,16 +11219,13 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilter *guestFilter, } if (virNetDevSetRcvMulti(ifname, true) < 0) { - VIR_WARN("Couldn't set multicast flag to 'on' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", ifname); + VIR_WARN("Couldn't set multicast flag to 'on' for device %s while responding to NIC_RX_FILTER_CHANGED", + ifname); } if (virNetDevSetRcvAllMulti(ifname, guestFilter->multicast.overflow) < 0) { - VIR_WARN("Couldn't set allmulticast flag to '%s' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't set allmulticast flag to '%s' for device %s while responding to NIC_RX_FILTER_CHANGED", virTristateSwitchTypeToString(virTristateSwitchFromBool(guestFilter->multicast.overflow)), ifname); } @@ -11244,15 +11233,12 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilter *guestFilter, case VIR_NETDEV_RX_FILTER_MODE_NONE: if (virNetDevSetRcvAllMulti(ifname, false) < 0) { - VIR_WARN("Couldn't set allmulticast flag to 'off' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", ifname); + VIR_WARN("Couldn't set allmulticast flag to 'off' for device %s while responding to NIC_RX_FILTER_CHANGED", + ifname); } if (virNetDevSetRcvMulti(ifname, false) < 0) { - VIR_WARN("Couldn't set multicast flag to 'off' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't set multicast flag to 'off' for device %s while responding to NIC_RX_FILTER_CHANGED", ifname); } break; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 9957f927f5..a8292a9782 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1318,8 +1318,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDef *def, tmp = virPCIDeviceAddressGetIOMMUGroupNum(hostAddr); if (tmp < 0) { - VIR_WARN("Can't look up isolation group for host device " - "%04x:%02x:%02x.%x, device won't be isolated", + VIR_WARN("Can't look up isolation group for host device %04x:%02x:%02x.%x, device won't be isolated", hostAddr->domain, hostAddr->bus, hostAddr->slot, hostAddr->function); return; @@ -1362,9 +1361,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDef *def, tmp = qemuDomainFindUnusedIsolationGroup(def); if (tmp == 0) { - VIR_WARN("Can't obtain usable isolation group for interface " - "configured to use hostdev-backed network '%s', " - "device won't be isolated", + VIR_WARN("Can't obtain usable isolation group for interface configured to use hostdev-backed network '%s', device won't be isolated", iface->data.network.name); return; } @@ -1564,8 +1561,7 @@ qemuDomainCollectPCIAddress(virDomainDef *def G_GNUC_UNUSED, if (!info->pciConnectFlags) { g_autofree char *addrStr = virPCIDeviceAddressAsString(&info->addr.pci); - VIR_WARN("qemuDomainDeviceCalculatePCIConnectFlags() thinks that the " - "device with PCI address %s should not have a PCI address", + VIR_WARN("qemuDomainDeviceCalculatePCIConnectFlags() thinks that the device with PCI address %s should not have a PCI address", addrStr ? addrStr : "(unknown)"); info->pciConnectFlags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 529e9fe3be..d5e2b37ed5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3740,14 +3740,12 @@ processNicRxFilterChangedEvent(virQEMUDriver *driver, } if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) { - VIR_WARN("NIC_RX_FILTER_CHANGED event received for " - "non-existent device %s in domain %s", + VIR_WARN("NIC_RX_FILTER_CHANGED event received for non-existent device %s in domain %s", devAlias, vm->def->name); goto endjob; } if (dev.type != VIR_DOMAIN_DEVICE_NET) { - VIR_WARN("NIC_RX_FILTER_CHANGED event received for " - "non-network device %s in domain %s", + VIR_WARN("NIC_RX_FILTER_CHANGED event received for non-network device %s in domain %s", devAlias, vm->def->name); goto endjob; } @@ -6390,8 +6388,8 @@ qemuDomainObjStart(virConnectPtr conn, return ret; } else if (ret < 0) { - VIR_WARN("Unable to restore from managed state %s. " - "Maybe the file is corrupted?", managed_save); + VIR_WARN("Unable to restore from managed state %s. Maybe the file is corrupted?", + managed_save); return ret; } else { VIR_WARN("Ignoring incomplete managed state %s", managed_save); diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index d8633c6b28..ba6e160a25 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1708,8 +1708,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw, if (!isConfidential && !usesUefiVarsDevice && supportsSecureBoot != requiresSMM) { - VIR_WARN("Firmware description '%s' has invalid set of features: " - "%s = %d, %s = %d, %s = %d (isConfidential = %d)", + VIR_WARN("Firmware description '%s' has invalid set of features: %s = %d, %s = %d, %s = %d (isConfidential = %d)", filename, qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_REQUIRES_SMM), requiresSMM, @@ -1720,8 +1719,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw, isConfidential); } if (hasEnrolledKeys && !supportsSecureBoot) { - VIR_WARN("Firmware description '%s' has invalid set of features: " - "%s = %d, %s = %d", + VIR_WARN("Firmware description '%s' has invalid set of features: %s = %d, %s = %d", filename, qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_SECURE_BOOT), supportsSecureBoot, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b7a282b96e..71a64b0879 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1358,8 +1358,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, goto cleanup; } } else { - VIR_WARN("setting bandwidth on interfaces of " - "type '%s' is not implemented yet", + VIR_WARN("setting bandwidth on interfaces of type '%s' is not implemented yet", virDomainNetTypeToString(actualType)); } } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ffffeea75c..2c36ce8332 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1867,8 +1867,8 @@ qemuMigrationSrcPostcopyFailed(virDomainObj *vm) virDomainObjIsFailedPostcopy(vm, vm->job)) return; - VIR_WARN("Migration of domain %s failed during post-copy; " - "leaving the domain paused", vm->def->name); + VIR_WARN("Migration of domain %s failed during post-copy; leaving the domain paused", + vm->def->name); vm->job->asyncPaused = true; virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, @@ -1899,8 +1899,8 @@ qemuMigrationDstPostcopyFailed(virDomainObj *vm) virDomainObjIsFailedPostcopy(vm, vm->job)) return; - VIR_WARN("Incoming migration of domain '%s' failed during post-copy; " - "leaving the domain running", vm->def->name); + VIR_WARN("Incoming migration of domain '%s' failed during post-copy; leaving the domain running", + vm->def->name); vm->job->asyncPaused = true; if (state == VIR_DOMAIN_RUNNING) { @@ -2532,8 +2532,7 @@ qemuMigrationAnyConnectionClosed(virDomainObj *vm, VIR_DEBUG("Migration protocol interrupted in post-copy mode"); postcopy = true; } else { - VIR_WARN("Migration of domain %s finished but we don't know if the " - "domain was successfully started on destination or not", + VIR_WARN("Migration of domain %s finished but we don't know if the domain was successfully started on destination or not", vm->def->name); } break; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 532bb885a3..7efd3f443a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1437,8 +1437,8 @@ qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitor *mon, } if (virJSONValueObjectGetBoolean(data, "connected", &connected) < 0) { - VIR_WARN("missing connected state for %s " - "in PR_MANAGER_STATUS_CHANGED event", name); + VIR_WARN("missing connected state for %s in PR_MANAGER_STATUS_CHANGED event", + name); return; } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ec71eaed87..8a0f5d30ac 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3852,8 +3852,7 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, goto cleanup; } if (ssf < SSF_WARNING_LEVEL) { - VIR_WARN("negotiation SSF %d lower than %d will be deprecated. " - "Please upgrade your ciphers.", + VIR_WARN("negotiation SSF %d lower than %d will be deprecated. Please upgrade your ciphers.", ssf, SSF_WARNING_LEVEL); } priv->is_secure = 1; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index e2967e5e1f..a983686e93 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1266,9 +1266,8 @@ static virNetMessage *virNetServerClientDispatchRead(virNetServerClient *client) } else if (!client->nrequests_warning && client->nrequests_max > 1) { client->nrequests_warning = true; - VIR_WARN("Client hit max requests limit %zd. This may result " - "in keep-alive timeouts. Consider tuning the " - "max_client_requests server parameter", client->nrequests); + VIR_WARN("Client hit max requests limit %zd. This may result in keep-alive timeouts. Consider tuning the max_client_requests server parameter", + client->nrequests); } virNetServerClientUpdateEvent(client); diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 05ab7ec2f9..e7cffa8e59 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -807,8 +807,7 @@ virSecurityDACSetOwnership(virSecurityManager *mgr, * not much we can do. XATTRs refcounting is fubar'ed and * the only option we have is warn users. */ if (virSecurityDACRestoreFileLabelInternal(mgr, src, path, remember) < 0) - VIR_WARN("Unable to restore label on '%s'. " - "XATTRs might have been left in inconsistent state.", + VIR_WARN("Unable to restore label on '%s'. XATTRs might have been left in inconsistent state.", NULLSTR(src ? src->path : path)); virErrorRestore(&origerr); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0824217f24..423dcd0bd9 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -953,9 +953,7 @@ virSecuritySELinuxGenLabel(virSecurityManager *mgr, if (data->alt_domain_context == NULL) { static bool warned; if (!warned) { - VIR_WARN("SELinux policy does not define a domain type for QEMU TCG. " - "Guest startup may be denied due to missing 'execmem' privilege " - "unless the 'virt_use_execmem' policy boolean is enabled"); + VIR_WARN("SELinux policy does not define a domain type for QEMU TCG. Guest startup may be denied due to missing 'execmem' privilege unless the 'virt_use_execmem' policy boolean is enabled"); warned = true; } baselabel = data->domain_context; @@ -1454,8 +1452,7 @@ virSecuritySELinuxSetFilecon(virSecurityManager *mgr, * not much we can do. XATTRs refcounting is fubar'ed and * the only option we have is warn users. */ if (virSecuritySELinuxRestoreFileLabel(mgr, path, remember, false) < 0) - VIR_WARN("Unable to restore label on '%s'. " - "XATTRs might have been left in inconsistent state.", + VIR_WARN("Unable to restore label on '%s'. XATTRs might have been left in inconsistent state.", path); virErrorRestore(&origerr); diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 99a68a6053..a55e94e764 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -306,8 +306,7 @@ virSecurityStackSetHostdevLabel(virSecurityManager *mgr, vm, dev, vroot) < 0) { - VIR_WARN("Unable to restore hostdev label after failed set label " - "call virDriver=%s driver=%s domain=%s hostdev=%p", + VIR_WARN("Unable to restore hostdev label after failed set label call virDriver=%s driver=%s domain=%s hostdev=%p", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name, dev); @@ -367,8 +366,7 @@ virSecurityStackSetAllLabel(virSecurityManager *mgr, vm, migrated, chardevStdioLogd) < 0) { - VIR_WARN("Unable to restore all labels after failed set label call " - "virDriver=%s driver=%s domain=%s migrated=%d", + VIR_WARN("Unable to restore all labels after failed set label call virDriver=%s driver=%s domain=%s migrated=%d", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name, migrated); @@ -422,8 +420,7 @@ virSecurityStackSetSavedStateLabel(virSecurityManager *mgr, if (virSecurityManagerRestoreSavedStateLabel(item->securityManager, vm, savefile) < 0) { - VIR_WARN("Unable to restore saved state label after failed set " - "label call virDriver=%s driver=%s savefile=%s", + VIR_WARN("Unable to restore saved state label after failed set label call virDriver=%s driver=%s savefile=%s", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), savefile); @@ -523,8 +520,7 @@ virSecurityStackSetDaemonSocketLabel(virSecurityManager *mgr, for (item = item->prev; item; item = item->prev) { if (virSecurityManagerClearSocketLabel(item->securityManager, vm) < 0) { - VIR_WARN("Unable to clear new daemon socket label after failed " - "set label call virDriver=%s driver=%s domain=%s", + VIR_WARN("Unable to clear new daemon socket label after failed set label call virDriver=%s driver=%s domain=%s", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name); @@ -551,8 +547,7 @@ virSecurityStackSetSocketLabel(virSecurityManager *mgr, for (item = item->prev; item; item = item->prev) { if (virSecurityManagerClearSocketLabel(item->securityManager, vm) < 0) { - VIR_WARN("Unable to clear new socket label after failed " - "set label call virDriver=%s driver=%s domain=%s", + VIR_WARN("Unable to clear new socket label after failed set label call virDriver=%s driver=%s domain=%s", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name); @@ -673,9 +668,7 @@ virSecurityStackSetImageLabel(virSecurityManager *mgr, vm, src, flags) < 0) { - VIR_WARN("Unable to restore image label after failed set label " - "call virDriver=%s driver=%s domain=%s src=%p (path=%s) " - "flags=0x%x", + VIR_WARN("Unable to restore image label after failed set label call virDriver=%s driver=%s domain=%s src=%p (path=%s) flags=0x%x", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name, src, NULLSTR(src->path), flags); @@ -746,8 +739,7 @@ virSecurityStackSetMemoryLabel(virSecurityManager *mgr, if (virSecurityManagerRestoreMemoryLabel(item->securityManager, vm, mem) < 0) { - VIR_WARN("Unable to restore memory label after failed set label " - "call virDriver=%s driver=%s domain=%s mem=%p", + VIR_WARN("Unable to restore memory label after failed set label call virDriver=%s driver=%s domain=%s mem=%p", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name, mem); @@ -794,8 +786,7 @@ virSecurityStackSetInputLabel(virSecurityManager *mgr, if (virSecurityManagerRestoreInputLabel(item->securityManager, vm, input) < 0) { - VIR_WARN("Unable to restore input label after failed set label " - "call virDriver=%s driver=%s domain=%s input=%p", + VIR_WARN("Unable to restore input label after failed set label call virDriver=%s driver=%s domain=%s input=%p", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name, input); @@ -904,8 +895,7 @@ virSecurityStackDomainSetChardevLabel(virSecurityManager *mgr, def, dev_source, chardevStdioLogd) < 0) { - VIR_WARN("Unable to restore chardev label after failed set label " - "call virDriver=%s driver=%s domain=%s dev_source=%p", + VIR_WARN("Unable to restore chardev label after failed set label call virDriver=%s driver=%s domain=%s dev_source=%p", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), def->name, dev_source); @@ -955,8 +945,7 @@ virSecurityStackSetTPMLabels(virSecurityManager *mgr, for (item = item->prev; item; item = item->prev) { if (virSecurityManagerRestoreTPMLabels(item->securityManager, vm, setTPMStateLabel) < 0) { - VIR_WARN("Unable to restore TPM label after failed set label " - "call virDriver=%s driver=%s domain=%s", + VIR_WARN("Unable to restore TPM label after failed set label call virDriver=%s driver=%s domain=%s", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), vm->name); @@ -1004,8 +993,7 @@ virSecurityStackDomainSetNetdevLabel(virSecurityManager *mgr, for (item = item->prev; item; item = item->prev) { if (virSecurityManagerRestoreNetdevLabel(item->securityManager, def, net) < 0) { - VIR_WARN("Unable to restore netdev label after failed set label " - "call virDriver=%s driver=%s domain=%s", + VIR_WARN("Unable to restore netdev label after failed set label call virDriver=%s driver=%s domain=%s", virSecurityManagerGetVirtDriver(mgr), virSecurityManagerGetDriver(item->securityManager), def->name); diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 29d544d349..25cad4a28d 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -331,8 +331,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDState *ptr, } virUUIDFormat(def->uuid, uuidstr); - VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom " - "config_opts from XML", def->name, uuidstr); + VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom config_opts from XML", + def->name, uuidstr); } ptr->starttime = time(0); diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index 2cb555a6e6..064b959bcb 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -69,8 +69,7 @@ virStorageBackendZFSVolModeNeeded(void) ret = virCommandRun(cmd, &exit_code); if ((ret < 0) || (exit_code != 2)) { - VIR_WARN("Command 'zfs get' either failed " - "to run or exited with unexpected status"); + VIR_WARN("Command 'zfs get' either failed to run or exited with unexpected status"); return ret; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index e2b41de1f2..78dc9f9f1c 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4042,8 +4042,8 @@ virStorageBackendFileSystemMountAddOptions(virCommand *cmd, virBufferAsprintf(&buf, "%s,", opts->options[i]); virUUIDFormat(def->uuid, uuidstr); - VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom " - "mount_opts from XML", def->name, uuidstr); + VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom mount_opts from XML", + def->name, uuidstr); } virBufferTrim(&buf, ","); diff --git a/src/storage_file/storage_file_probe.c b/src/storage_file/storage_file_probe.c index 82cea28b20..a1f4c25b3d 100644 --- a/src/storage_file/storage_file_probe.c +++ b/src/storage_file/storage_file_probe.c @@ -883,8 +883,7 @@ virStorageFileProbeFormatFromBuf(const char *path, } if (possibleFormat != VIR_STORAGE_FILE_RAW) - VIR_WARN("File %s matches %s magic, but version is wrong. " - "Please report new version to devel@lists.libvirt.org", + VIR_WARN("File %s matches %s magic, but version is wrong. Please report new version to devel@lists.libvirt.org", path, virStorageFileFormatTypeToString(possibleFormat)); cleanup: diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 882f9e1f01..65ae9cc651 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -2099,8 +2099,8 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags if (NS_SUCCEEDED(rc)) gVBoxAPI.deleteConfig(machine); else - VIR_WARN("Could not cleanup partially created VM after failure, " - "rc=%08x", rc); + VIR_WARN("Could not cleanup partially created VM after failure, rc=%08x", + rc); } VBOX_RELEASE(machine); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 34214b4bf3..084b415442 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2110,8 +2110,7 @@ virVMXParseVNC(virConf *conf, virDomainGraphicsDef **def) VIR_FREE(listenAddr); if (port < 0) { - VIR_WARN("VNC is enabled but VMX entry 'RemoteDisplay.vnc.port' " - "is missing, the VNC port is unknown"); + VIR_WARN("VNC is enabled but VMX entry 'RemoteDisplay.vnc.port' is missing, the VNC port is unknown"); (*def)->data.vnc.port = 0; (*def)->data.vnc.autoport = true; @@ -3879,8 +3878,7 @@ virVMXFormatVNC(virDomainGraphicsDef *def, virBuffer *buffer) virBufferAddLit(buffer, "RemoteDisplay.vnc.enabled = \"true\"\n"); if (def->data.vnc.autoport) { - VIR_WARN("VNC autoport is enabled, but the automatically assigned " - "VNC port cannot be read back"); + VIR_WARN("VNC autoport is enabled, but the automatically assigned VNC port cannot be read back"); } else { if (def->data.vnc.port < 5900 || def->data.vnc.port > 5964) { VIR_WARN("VNC port %d it out of [5900..5964] range", diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index e6dd9533be..7059f359b6 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3218,8 +3218,7 @@ static int prlsdkConfigureNet(struct _vzDriver *driver G_GNUC_UNUSED, if (isCt) { if (net->model != VIR_DOMAIN_NET_MODEL_UNKNOWN) - VIR_WARN("Setting network adapter for containers is not " - "supported by vz driver."); + VIR_WARN("Setting network adapter for containers is not supported by vz driver."); } else { if (net->model == VIR_DOMAIN_NET_MODEL_RTL8139) { pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> Since VIR_WARN doesn't use translatable messages, the 'sc_prohibit_error_message_on_multiple_lines' check doesn't catch those. Introduce another check for VIR_WARN. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- build-aux/syntax-check.mk | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 022e8e6550..6e22212944 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -478,6 +478,14 @@ sc_prohibit_error_message_on_multiple_lines: halt='found error message on multiple lines' \ $(_sc_search_regexp) +# Disallow VIR_WARN and other messagess on multiple lines, except when +# they end with '\n'. +sc_prohibit_warnings_on_multiple_lines: + @prohibit='VIR_WARN\("[^"]*"$$' \ + exclude='\\n"$$' \ + halt='found error message on multiple lines' \ + $(_sc_search_regexp) + # Look for diagnostics that lack a % in the format string, except that we # allow VIR_ERROR to do this, and ignore functions that take a single # string rather than a format argument. -- 2.54.0
(I prefer the way you describe it in the actual commit message - "don't break up lines in the source" - vs the subject line here - "prohibit newlines" - since it's more accurate. Fortunately the actual commit message is used as .... the actual commit message, so everything is cool! :-)) On 5/11/26 12:23 PM, Peter Krempa via Devel wrote:
Make them the same as we do with error messages since they often end up in logs.
Since they are not translated [1] the existing check didn't catch that.
[1] IMO in some cases they are in fact used as errors, and maybe would be worth to be translated. But this is for another series
There have been a few times I've wished we were allowed to at least *optionally* mark VIR_WARN strings to be translated, for cases where the situation is something like: "This is *probably* going to lead to an error, and we've just seen evidence of it in advance of the place where it's going to actually blow up. Right now while we have this context we can tell the user how to fix it (and it would be nice to tell them in a language they understand), but it would be awkward and wasteful to try and regather that context later at the time we actually fail. But also it might *not* be an error, so we don't want to just fail right away in case we wouldn't have failed later." or something like that. For example (a recent one), we determine that a particular default path for a directory for something isn't accessible by the uid of the process running libvirt, but it turns out that in some situations that path is never actually used, and so adding an actual error when we decide on the path for config/logging/status would cause some working setups to "mysteriously" start failing. (On the other hand, it's awkward to later regather the context that the reason for the open/create failure was because "the log path is inaccessible by this process" (the fact that this was the source of the file path is usually a few layers up the call stack from where the error is logged). So yeah, that issue is completely unrelated to your patch, but I thought I'd chime in while the topic was brought up. Reviewed-by: Laine Stump <laine@redhat.com> for the series.
Peter Krempa (2): Don't break up strings for VIR_WARN messages syntax-check: Enforce no linebreaks in VIR_WARN messages
build-aux/syntax-check.mk | 8 +++++ src/bhyve/bhyve_command.c | 4 +-- src/conf/virdomainjob.c | 5 +-- src/cpu/cpu_x86.c | 5 ++- src/esx/esx_vi.c | 3 +- src/hypervisor/virhostdev.c | 12 +++---- src/interface/interface_backend_netcf.c | 12 +++---- src/libxl/libxl_conf.c | 4 +-- src/libxl/xen_xl.c | 4 +-- src/lxc/lxc_driver.c | 3 +- src/lxc/lxc_process.c | 3 +- src/network/bridge_driver.c | 15 +++------ src/nwfilter/nwfilter_dhcpsnoop.c | 3 +- src/qemu/qemu_block.c | 3 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_conf.c | 4 +-- src/qemu/qemu_domain.c | 44 +++++++++---------------- src/qemu/qemu_domain_address.c | 10 ++---- src/qemu/qemu_driver.c | 10 +++--- src/qemu/qemu_firmware.c | 6 ++-- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_migration.c | 11 +++---- src/qemu/qemu_monitor_json.c | 4 +-- src/remote/remote_driver.c | 3 +- src/rpc/virnetserverclient.c | 5 ++- src/security/security_dac.c | 3 +- src/security/security_selinux.c | 7 ++-- src/security/security_stack.c | 34 +++++++------------ src/storage/storage_backend_rbd.c | 4 +-- src/storage/storage_backend_zfs.c | 3 +- src/storage/storage_util.c | 4 +-- src/storage_file/storage_file_probe.c | 3 +- src/vbox/vbox_common.c | 4 +-- src/vmx/vmx.c | 6 ++-- src/vz/vz_sdk.c | 3 +- 35 files changed, 96 insertions(+), 162 deletions(-)
On Mon, May 11, 2026 at 17:32:44 -0400, Laine Stump wrote:
(I prefer the way you describe it in the actual commit message - "don't break up lines in the source" - vs the subject line here - "prohibit newlines" - since it's more accurate. Fortunately the actual commit message is used as .... the actual commit message, so everything is cool! :-))
On 5/11/26 12:23 PM, Peter Krempa via Devel wrote:
Make them the same as we do with error messages since they often end up in logs.
Since they are not translated [1] the existing check didn't catch that.
[1] IMO in some cases they are in fact used as errors, and maybe would be worth to be translated. But this is for another series
There have been a few times I've wished we were allowed to at least *optionally* mark VIR_WARN strings to be translated, for cases where the situation is something like:
Yeah, at least some of the warnings we emit would benefit from a translation. Especially since they are logged by default.
"This is *probably* going to lead to an error, and we've just seen evidence of it in advance of the place where it's going to actually blow up. Right now while we have this context we can tell the user how to fix it (and it would be nice to tell them in a language they understand), but it would be awkward and wasteful to try and regather that context later at the time we actually fail. But also it might *not* be an error, so we don't want to just fail right away in case we wouldn't have failed later."
or something like that. For example (a recent one), we determine that a particular default path for a directory for something isn't accessible by the uid of the process running libvirt, but it turns out that in some situations that path is never actually used, and so adding an actual error when we decide on the path for config/logging/status would cause some working setups to "mysteriously" start failing. (On the other hand, it's awkward to later regather the context that the reason for the open/create failure was because "the log path is inaccessible by this process" (the fact that this was the source of the file path is usually a few layers up the call stack from where the error is logged).
So yeah, that issue is completely unrelated to your patch, but I thought I'd chime in while the topic was brought up.
Well, this is a first step :). Maybe Jirka still has his script to add positional substitutions which he used when adding them to virReportError, so we could also do the second step too :)
Reviewed-by: Laine Stump <laine@redhat.com>
Thanks!
participants (2)
-
Laine Stump -
Peter Krempa