10:59 a.m.
Replace VIR_ERROR logging macros for error reporting in driver startup routines
with vir*Error function. Don't use virStrerror and remove ebuf.
Link to task:
http://wiki.libvirt.org/page/BiteSizedTasks#Replace_VIR_ERROR_with_standa...
Jovanka Gulicoska (4):
uml: Replace VIR_ERROR with standard vir*Error in state driver init
qemu: Replace VIR_ERROR with standard vir*Error in state driver init
xen: Replace VIR_ERROR with standard vir*Error in state driver init
node_device: Replace VIR_ERROR with standard vir*Error in state driver
init
src/node_device/node_device_hal.c | 26 +++++++++++-------
src/node_device/node_device_udev.c | 49 ++++++++++++++++++++--------------
src/qemu/qemu_driver.c | 54 +++++++++++++++++++++++---------------
src/uml/uml_driver.c | 20 ++++++--------
src/xen/xen_driver.c | 5 ++--
5 files changed, 90 insertions(+), 64 deletions(-)
--
2.5.5
10:59 a.m.
New subject: [libvirt] [PATCH v3 1/4] uml: Replace VIR_ERROR with standard vir*Error in state driver init
---
src/uml/uml_driver.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index a674c12..8dd54d7 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -188,8 +188,8 @@ umlAutostartDomain(virDomainObjPtr vm,
ret = umlStartVMDaemon(data->conn, data->driver, vm, false);
virDomainAuditStart(vm, "booted", ret >= 0);
if (ret < 0) {
- VIR_ERROR(_("Failed to autostart VM '%s': %s"),
- vm->def->name, virGetLastErrorMessage());
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart VM
'%s': %s"),
+ vm->def->name, virGetLastErrorMessage());
} else {
virObjectEventPtr event =
virDomainEventLifecycleNewFromObj(vm,
@@ -535,15 +535,13 @@ umlStateInitialize(bool privileged,
goto error;
if ((uml_driver->inotifyFD = inotify_init()) < 0) {
- VIR_ERROR(_("cannot initialize inotify"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize
inotify"));
goto error;
}
if (virFileMakePath(uml_driver->monitorDir) < 0) {
- char ebuf[1024];
- VIR_ERROR(_("Failed to create monitor directory %s: %s"),
- uml_driver->monitorDir,
- virStrerror(errno, ebuf, sizeof(ebuf)));
+ virReportSystemError(errno, _("Failed to create monitor directory
%s"),
+ uml_driver->monitorDir);
goto error;
}
@@ -551,10 +549,8 @@ umlStateInitialize(bool privileged,
if (inotify_add_watch(uml_driver->inotifyFD,
uml_driver->monitorDir,
IN_CREATE | IN_MODIFY | IN_DELETE) < 0) {
- char ebuf[1024];
- VIR_ERROR(_("Failed to create inotify watch on %s: %s"),
- uml_driver->monitorDir,
- virStrerror(errno, ebuf, sizeof(ebuf)));
+ virReportSystemError(errno, _("Failed to create inotify watch on %s"),
+ uml_driver->monitorDir);
goto error;
}
@@ -582,7 +578,7 @@ umlStateInitialize(bool privileged,
return 0;
out_of_memory:
- VIR_ERROR(_("umlStartup: out of memory"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("umlStartup: out of
memory"));
error:
VIR_FREE(userdir);
--
2.5.5
10:59 a.m.
New subject: [libvirt] [PATCH v3 2/4] qemu: Replace VIR_ERROR with standard vir*Error in state driver init
---
src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 10d3e3d..85a116a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -290,15 +290,17 @@ qemuAutostartDomain(virDomainObjPtr vm,
if (vm->autostart &&
!virDomainObjIsActive(vm)) {
if (qemuProcessBeginJob(data->driver, vm) < 0) {
- VIR_ERROR(_("Failed to start job on VM '%s': %s"),
- vm->def->name, virGetLastErrorMessage());
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to start job on VM '%s': %s"),
+ vm->def->name, virGetLastErrorMessage());
goto cleanup;
}
if (qemuDomainObjStart(data->conn, data->driver, vm, flags,
QEMU_ASYNC_JOB_START) < 0) {
- VIR_ERROR(_("Failed to autostart VM '%s': %s"),
- vm->def->name, virGetLastErrorMessage());
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to autostart VM '%s': %s"),
+ vm->def->name, virGetLastErrorMessage());
}
qemuProcessEndJob(data->driver, vm);
@@ -450,7 +452,8 @@ qemuSecurityInit(virQEMUDriverPtr driver)
return 0;
error:
- VIR_ERROR(_("Failed to initialize security drivers"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to initialize security drivers"));
virObjectUnref(stack);
virObjectUnref(mgr);
virObjectUnref(cfg);
@@ -471,7 +474,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
virDomainSnapshotDefPtr def = NULL;
virDomainSnapshotObjPtr snap = NULL;
virDomainSnapshotObjPtr current = NULL;
- char ebuf[1024];
unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE |
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);
@@ -481,8 +483,10 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
virObjectLock(vm);
if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) <
0) {
- VIR_ERROR(_("Failed to allocate memory for snapshot directory for domain
%s"),
- vm->def->name);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to allocate memory for "
+ "snapshot directory for domain %s"),
+ vm->def->name);
goto cleanup;
}
@@ -494,9 +498,10 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
if (!(dir = opendir(snapDir))) {
if (errno != ENOENT)
- VIR_ERROR(_("Failed to open snapshot directory %s for domain %s:
%s"),
- snapDir, vm->def->name,
- virStrerror(errno, ebuf, sizeof(ebuf)));
+ virReportSystemError(errno,
+ _("Failed to open snapshot directory %s "
+ "for domain %s"),
+ snapDir, vm->def->name);
goto cleanup;
}
@@ -509,14 +514,16 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
VIR_INFO("Loading snapshot file '%s'", entry->d_name);
if (virAsprintf(&fullpath, "%s/%s", snapDir, entry->d_name) <
0) {
- VIR_ERROR(_("Failed to allocate memory for path"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to allocate memory for path"));
continue;
}
if (virFileReadAll(fullpath, 1024*1024*1, &xmlStr) < 0) {
/* Nothing we can do here, skip this one */
- VIR_ERROR(_("Failed to read snapshot file %s: %s"), fullpath,
- virStrerror(errno, ebuf, sizeof(ebuf)));
+ virReportSystemError(errno,
+ _("Failed to read snapshot file %s"),
+ fullpath);
VIR_FREE(fullpath);
continue;
}
@@ -526,8 +533,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
flags);
if (def == NULL) {
/* Nothing we can do here, skip this one */
- VIR_ERROR(_("Failed to parse snapshot XML from file
'%s'"),
- fullpath);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to parse snapshot XML from file
'%s'"),
+ fullpath);
VIR_FREE(fullpath);
VIR_FREE(xmlStr);
continue;
@@ -546,17 +554,21 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
VIR_FREE(xmlStr);
}
if (direrr < 0)
- VIR_ERROR(_("Failed to fully read directory %s"), snapDir);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to fully read directory %s"),
+ snapDir);
if (vm->current_snapshot != current) {
- VIR_ERROR(_("Too many snapshots claiming to be current for domain
%s"),
- vm->def->name);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Too many snapshots claiming to be current for domain
%s"),
+ vm->def->name);
vm->current_snapshot = NULL;
}
if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0)
- VIR_ERROR(_("Snapshots have inconsistent relations for domain %s"),
- vm->def->name);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Snapshots have inconsistent relations for domain
%s"),
+ vm->def->name);
/* FIXME: qemu keeps internal track of snapshots. We can get access
* to this info via the "info snapshots" monitor command for running
--
2.5.5
10:59 a.m.
New subject: [libvirt] [PATCH v3 3/4] xen: Replace VIR_ERROR with standard vir*Error in state driver init
---
src/xen/xen_driver.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 3f5d80d..04c0809 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -407,7 +407,6 @@ static virDrvOpenStatus
xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
{
xenUnifiedPrivatePtr priv;
- char ebuf[1024];
/*
* Only the libvirtd instance can open this driver.
@@ -532,8 +531,8 @@ xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
unsigned int f
goto error;
if (virFileMakePath(priv->saveDir) < 0) {
- VIR_ERROR(_("Errored to create save dir '%s': %s"),
priv->saveDir,
- virStrerror(errno, ebuf, sizeof(ebuf)));
+ virReportSystemError(errno, _("Errored to create save dir
'%s'"),
+ priv->saveDir);
goto error;
}
--
2.5.5
10:59 a.m.
New subject: [libvirt] [PATCH v3 4/4] node_device: Replace VIR_ERROR with standard vir*Error in state driver init
---
src/node_device/node_device_hal.c | 26 +++++++++++++-------
src/node_device/node_device_udev.c | 49 +++++++++++++++++++++++---------------
2 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index 6ddfad0..7a9c3d9 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -641,24 +641,29 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED,
dbus_error_init(&err);
if (!(sysbus = virDBusGetSystemBus())) {
- VIR_ERROR(_("DBus not available, disabling HAL driver: %s"),
- virGetLastErrorMessage());
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("DBus not available, disabling HAL driver: %s"),
+ virGetLastErrorMessage());
ret = 0;
goto failure;
}
hal_ctx = libhal_ctx_new();
if (hal_ctx == NULL) {
- VIR_ERROR(_("libhal_ctx_new returned NULL"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libhal_ctx_new returned NULL"));
goto failure;
}
if (!libhal_ctx_set_dbus_connection(hal_ctx, sysbus)) {
- VIR_ERROR(_("libhal_ctx_set_dbus_connection failed"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libhal_ctx_set_dbus_connection failed"));
goto failure;
}
if (!libhal_ctx_init(hal_ctx, &err)) {
- VIR_ERROR(_("libhal_ctx_init failed, haldaemon is probably not
running"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libhal_ctx_init failed, haldaemon "
+ "is probably not running"));
/* We don't want to show a fatal error here,
otherwise entire libvirtd shuts down when
hald isn't running */
@@ -683,13 +688,15 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED,
!libhal_ctx_set_device_lost_capability(hal_ctx, device_cap_lost) ||
!libhal_ctx_set_device_property_modified(hal_ctx, device_prop_modified) ||
!libhal_device_property_watch_all(hal_ctx, &err)) {
- VIR_ERROR(_("setting up HAL callbacks failed"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("setting up HAL callbacks failed"));
goto failure;
}
udi = libhal_get_all_devices(hal_ctx, &num_devs, &err);
if (udi == NULL) {
- VIR_ERROR(_("libhal_get_all_devices failed"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libhal_get_all_devices failed"));
goto failure;
}
for (i = 0; i < num_devs; i++) {
@@ -702,7 +709,7 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED,
failure:
if (dbus_error_is_set(&err)) {
- VIR_ERROR(_("%s: %s"), err.name, err.message);
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: %s"), err.name,
err.message);
dbus_error_free(&err);
}
virNodeDeviceObjListFree(&driver->devs);
@@ -753,7 +760,8 @@ nodeStateReload(void)
dbus_error_init(&err);
udi = libhal_get_all_devices(hal_ctx, &num_devs, &err);
if (udi == NULL) {
- VIR_ERROR(_("libhal_get_all_devices failed"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libhal_get_all_devices failed"));
return -1;
}
for (i = 0; i < num_devs; i++) {
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 7d111c4..3315153 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -66,7 +66,8 @@ static int udevStrToLong_ull(char const *s,
ret = virStrToLong_ull(s, end_ptr, base, result);
if (ret != 0) {
- VIR_ERROR(_("Failed to convert '%s' to unsigned long long"),
s);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to convert '%s' to unsigned long
long"), s);
} else {
VIR_DEBUG("Converted '%s' to unsigned long %llu", s, *result);
}
@@ -84,7 +85,8 @@ static int udevStrToLong_ui(char const *s,
ret = virStrToLong_ui(s, end_ptr, base, result);
if (ret != 0) {
- VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to convert '%s' to unsigned int"),
s);
} else {
VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result);
}
@@ -101,7 +103,8 @@ static int udevStrToLong_i(char const *s,
ret = virStrToLong_i(s, end_ptr, base, result);
if (ret != 0) {
- VIR_ERROR(_("Failed to convert '%s' to int"), s);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to convert '%s' to int"), s);
} else {
VIR_DEBUG("Converted '%s' to int %u", s, *result);
}
@@ -717,8 +720,9 @@ static int udevProcessSCSIHost(struct udev_device *device
ATTRIBUTE_UNUSED,
filename = last_component(def->sysfs_path);
if (!(str = STRSKIP(filename, "host"))) {
- VIR_ERROR(_("SCSI host found, but its udev name '%s' does "
- "not begin with 'host'"), filename);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("SCSI host found, but its udev name '%s' does
"
+ "not begin with 'host'"), filename);
goto out;
}
@@ -875,8 +879,9 @@ static int udevProcessSCSIDevice(struct udev_device *device
ATTRIBUTE_UNUSED,
out:
if (ret != 0) {
- VIR_ERROR(_("Failed to process SCSI device with sysfs path
'%s'"),
- def->sysfs_path);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to process SCSI device with sysfs path
'%s'"),
+ def->sysfs_path);
}
return ret;
}
@@ -1293,7 +1298,8 @@ static int udevGetDeviceDetails(struct udev_device *device,
ret = udevProcessSCSIGeneric(device, def);
break;
default:
- VIR_ERROR(_("Unknown device type %d"), def->caps->data.type);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown device type %d"),
def->caps->data.type);
ret = -1;
break;
}
@@ -1459,7 +1465,8 @@ static int udevEnumerateDevices(struct udev *udev)
ret = udev_enumerate_scan_devices(udev_enumerate);
if (0 != ret) {
- VIR_ERROR(_("udev scan devices returned %d"), ret);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("udev scan devices returned %d"), ret);
goto out;
}
@@ -1539,14 +1546,16 @@ static void udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
nodeDeviceLock();
udev_fd = udev_monitor_get_fd(udev_monitor);
if (fd != udev_fd) {
- VIR_ERROR(_("File descriptor returned by udev %d does not "
- "match node device file descriptor %d"), fd, udev_fd);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("File descriptor returned by udev %d does not "
+ "match node device file descriptor %d"), fd, udev_fd);
goto out;
}
device = udev_monitor_receive_device(udev_monitor);
if (device == NULL) {
- VIR_ERROR(_("udev_monitor_receive_device returned NULL"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("udev_monitor_receive_device returned NULL"));
goto out;
}
@@ -1584,8 +1593,9 @@ udevGetDMIData(virNodeDevCapDataPtr data)
if (device == NULL) {
device = udev_device_new_from_syspath(udev, DMI_DEVPATH_FALLBACK);
if (device == NULL) {
- VIR_ERROR(_("Failed to get udev device for syspath '%s' or
'%s'"),
- DMI_DEVPATH, DMI_DEVPATH_FALLBACK);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get udev device for syspath '%s' or
'%s'"),
+ DMI_DEVPATH, DMI_DEVPATH_FALLBACK);
goto out;
}
}
@@ -1691,9 +1701,8 @@ static int udevPCITranslateInit(bool privileged ATTRIBUTE_UNUSED)
* situation, but a non-privileged user won't benefit much
* from udev in the first place. */
if (errno != ENOENT && (privileged || errno != EACCES)) {
- char ebuf[256];
- VIR_ERROR(_("Failed to initialize libpciaccess: %s"),
- virStrerror(pciret, ebuf, sizeof(ebuf)));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to initialize libpciaccess: %s"));
return -1;
}
}
@@ -1721,7 +1730,8 @@ static int nodeStateInitialize(bool privileged,
}
if (virMutexInit(&driver->lock) < 0) {
- VIR_ERROR(_("Failed to initialize mutex for driver"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to initialize mutex for driver"));
VIR_FREE(priv);
VIR_FREE(driver);
return -1;
@@ -1747,7 +1757,8 @@ static int nodeStateInitialize(bool privileged,
priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
if (priv->udev_monitor == NULL) {
- VIR_ERROR(_("udev_monitor_new_from_netlink returned NULL"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("udev_monitor_new_from_netlink returned NULL"));
goto out_unlock;
}
--
2.5.5
1:13 p.m.
New subject: [libvirt] [PATCH v3 4/4] node_device: Replace VIR_ERROR with standard vir*Error in state driver init
On 06/07/2016 11:59 AM, Jovanka Gulicoska wrote:
---
src/node_device/node_device_hal.c | 26 +++++++++++++-------
src/node_device/node_device_udev.c | 49 +++++++++++++++++++++++---------------
2 files changed, 47 insertions(+), 28 deletions(-)
I pushed patches 1-3, thanks!
This patch needs to be rebased on master, since node_device_udev had a bunch
of cleanups recently. There's also one issue I noticed:
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 7d111c4..3315153 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1691,9 +1701,8 @@ static int udevPCITranslateInit(bool privileged
ATTRIBUTE_UNUSED)
* situation, but a non-privileged user won't benefit much
* from udev in the first place. */
if (errno != ENOENT && (privileged || errno != EACCES)) {
- char ebuf[256];
- VIR_ERROR(_("Failed to initialize libpciaccess: %s"),
- virStrerror(pciret, ebuf, sizeof(ebuf)));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to initialize libpciaccess: %s"));
return -1;
}
}
I think you forgot to preserve the virStrerror call here, otherwise the string
format is incorrect, with the trailing ": %s"
Thanks,
Cole
3090
days inactive
3090
days old
5 comments
2 participants
participants (2)
-
Cole Robinson -
Jovanka Gulicoska