Various error reporting fixes

- Don't duplicate SystemError - Use proper error code in domain_conf - Fix a broken error call in qemu_conf - Don't use VIR_ERR_ERROR in security driver (isn't a valid code in this case)
2024-07-11 04:15:49 +00:00 · 2009-10-27 12:20:22 -04:00 · 2009-10-27 12:20:22 -04:00 · 4c44cdcce3
commit 4c44cdcce3
parent 517761fd96
9 changed files with 30 additions and 34 deletions
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@ -3705,7 +3705,7 @@ virDomainCpuSetParse(virConnectPtr conn, const char **str, char sep,
    return (ret);

  parse_error:
-    virDomainReportError(conn, VIR_ERR_XEN_CALL,
+    virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                         "%s", _("topology cpuset syntax error"));
    return (-1);
 }
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@ -1593,9 +1593,9 @@ virStoragePoolObjSaveDef(virConnectPtr conn,
        char path[PATH_MAX];

        if ((err = virFileMakePath(driver->configDir))) {
-            virStorageReportError(conn, err,
-                                  _("cannot create config directory %s"),
-                                  driver->configDir);
+            virReportSystemError(conn, err,
+                                 _("cannot create config directory %s"),
+                                 driver->configDir);
            return -1;
        }

--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@ -1106,10 +1106,8 @@ int qemudExtractVersion(virConnectPtr conn,
        return -1;

    if (stat(binary, &sb) < 0) {
-        char ebuf[1024];
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                         _("Cannot find QEMU binary %s: %s"), binary,
-                         virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("Cannot find QEMU binary %s"), binary);
        return -1;
    }

--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@ -1771,7 +1771,7 @@ static int qemuDomainSetHostdevOwnership(virConnectPtr conn,
    }
    return 0;
 #else
-    qemudReportError(conn, NULL, NULL, "%s",
+    qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s",
                     _("unable to set host device ownership on this platform"));
    return -1;
 #endif
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@ -112,7 +112,7 @@ profile_status_file(const char *str)

    if (snprintf(profile, PATH_MAX, "%s/%s", APPARMOR_DIR "/libvirt", str)
       > PATH_MAX - 1) {
-        virSecurityReportError(NULL, VIR_ERR_ERROR,
+        virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("profile name exceeds maximum length"));
    }

@ -204,7 +204,7 @@ load_profile(virConnectPtr conn, const char *profile, virDomainObjPtr vm,
        if (errno == EINTR)
            goto rewait;

-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("Unexpected exit status from virt-aa-helper "
                               "%d pid %lu"),
                               WEXITSTATUS(status), (unsigned long)child);
@ -265,7 +265,7 @@ use_apparmor(void)

    if ((len = readlink("/proc/self/exe", libvirt_daemon,
                        PATH_MAX - 1)) < 0) {
-        virSecurityReportError(NULL, VIR_ERR_ERROR,
+        virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("could not find libvirtd"));
        return rc;
    }
@ -289,13 +289,13 @@ AppArmorSecurityDriverProbe(void)
    /* see if template file exists */
    if (snprintf(template, PATH_MAX, "%s/TEMPLATE",
                 APPARMOR_DIR "/libvirt") > PATH_MAX - 1) {
-        virSecurityReportError(NULL, VIR_ERR_ERROR,
+        virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("template too large"));
        return SECURITY_DRIVER_DISABLE;
    }

    if (!virFileExists(template)) {
-        virSecurityReportError(NULL, VIR_ERR_ERROR,
+        virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR,
                               _("template \'%s\' does not exist"), template);
        return SECURITY_DRIVER_DISABLE;
    }
@ -326,7 +326,7 @@ AppArmorGenSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)

    if ((vm->def->seclabel.label) ||
        (vm->def->seclabel.model) || (vm->def->seclabel.imagelabel)) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s",
                               _("security label already defined for VM"));
        return rc;
@ -338,7 +338,7 @@ AppArmorGenSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)
    /* if the profile is not already loaded, then load one */
    if (profile_loaded(profile_name) < 0) {
        if (load_profile(conn, profile_name, vm, NULL) < 0) {
-            virSecurityReportError(conn, VIR_ERR_ERROR,
+            virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                   _("cannot generate AppArmor profile "
                                   "\'%s\'"), profile_name);
            goto clean;
@ -395,13 +395,13 @@ AppArmorGetSecurityLabel(virConnectPtr conn,

    if (virStrcpy(sec->label, profile_name,
        VIR_SECURITY_LABEL_BUFLEN) == NULL) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("error copying profile name"));
        goto clean;
    }

    if ((sec->enforcing = profile_status(profile_name, 1)) < 0) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("error calling profile_status()"));
        goto clean;
    }
@ -424,7 +424,7 @@ AppArmorRestoreSecurityLabel(virConnectPtr conn, virDomainObjPtr vm)

    if (secdef->imagelabel) {
        if ((rc = remove_profile(secdef->label)) != 0) {
-            virSecurityReportError(conn, VIR_ERR_ERROR,
+            virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                   _("could not remove profile for \'%s\'"),
                                   secdef->label);
        }
@ -450,7 +450,7 @@ AppArmorSetSecurityLabel(virConnectPtr conn,
        return rc;

    if (STRNEQ(drv->name, secdef->model)) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("security label driver mismatch: "
                               "\'%s\' model configured for domain, but "
                               "hypervisor driver is \'%s\'."),
@ -460,7 +460,7 @@ AppArmorSetSecurityLabel(virConnectPtr conn,
    }

    if (aa_change_profile(profile_name) < 0) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("error calling aa_change_profile()"));
        goto clean;
    }
@ -490,7 +490,7 @@ AppArmorRestoreSecurityImageLabel(virConnectPtr conn,
        /* Update the profile only if it is loaded */
        if (profile_loaded(secdef->imagelabel) >= 0) {
            if (load_profile(conn, secdef->imagelabel, vm, NULL) < 0) {
-                virSecurityReportError(conn, VIR_ERR_ERROR,
+                virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                       _("cannot update AppArmor profile "
                                       "\'%s\'"),
                                       secdef->imagelabel);
@ -520,7 +520,7 @@ AppArmorSetSecurityImageLabel(virConnectPtr conn,
    if (secdef->imagelabel) {
        /* if the device doesn't exist, error out */
        if (!virFileExists(disk->src)) {
-            virSecurityReportError(conn, VIR_ERR_ERROR,
+            virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                   _("\'%s\' does not exist"), disk->src);
            return rc;
        }
@ -531,7 +531,7 @@ AppArmorSetSecurityImageLabel(virConnectPtr conn,
        /* update the profile only if it is loaded */
        if (profile_loaded(secdef->imagelabel) >= 0) {
            if (load_profile(conn, secdef->imagelabel, vm, disk) < 0) {
-                virSecurityReportError(conn, VIR_ERR_ERROR,
+                virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                     _("cannot update AppArmor profile "
                                     "\'%s\'"),
                                     secdef->imagelabel);
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@ -123,7 +123,7 @@ virSecurityDriverSetDOI(virConnectPtr conn,
                        const char *doi)
 {
    if (strlen(doi) >= VIR_SECURITY_DOI_BUFLEN) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("%s: DOI \'%s\' is "
                               "longer than the maximum allowed length of %d"),
                               __func__, doi, VIR_SECURITY_DOI_BUFLEN - 1);
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@ -189,13 +189,13 @@ SELinuxGenSecurityLabel(virConnectPtr conn,

    vm->def->seclabel.label = SELinuxGenNewContext(default_domain_context, mcs);
    if (! vm->def->seclabel.label)  {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("cannot generate selinux context for %s"), mcs);
        goto err;
    }
    vm->def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs);
    if (! vm->def->seclabel.imagelabel)  {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("cannot generate selinux context for %s"), mcs);
        goto err;
    }
@ -285,9 +285,9 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
    }

    if (strlen((char *) ctx) >= VIR_SECURITY_LABEL_BUFLEN) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("security label exceeds "
-                                 "maximum lenth: %d"),
+                                 "maximum length: %d"),
                               VIR_SECURITY_LABEL_BUFLEN - 1);
        return -1;
    }
@ -647,7 +647,7 @@ SELinuxSetSecurityLabel(virConnectPtr conn,
    int i;

    if (!STREQ(drv->name, secdef->model)) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
+        virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("security label driver mismatch: "
                                 "'%s' model configured for domain, but "
                                 "hypervisor driver is '%s'."),
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@ -2298,9 +2298,7 @@ get_cpu_flags(virConnectPtr conn, const char **hvm, int *pae, int *longmode)

    if ((fd = open("/dev/cpu/self/cpuid", O_RDONLY)) == -1 ||
        pread(fd, &regs, sizeof(regs), 0) != sizeof(regs)) {
-        char ebuf[1024];
-        virXenError(conn, VIR_ERR_SYSTEM_ERROR,
-            "couldn't read CPU flags: %s", virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno, "%s", _("could not read CPU flags"));
        goto out;
    }

--- a/tests/cpuset
+++ b/tests/cpuset
@ -41,7 +41,7 @@ sed "s/vcpu>/vcpu cpuset='aaa'>/" xml > xml-invalid || fail=1
 $abs_top_builddir/tools/virsh --connect test:///default define xml-invalid > out 2>&1 && fail=1
 cat <<\EOF > exp || fail=1
 error: Failed to define domain from xml-invalid
-error: failed Xen syscall topology cpuset syntax error
+error: internal error topology cpuset syntax error

 EOF
 compare exp out || fail=1