Re: [PATCH] hw/misc: deprecate the 'sga' device
On Thu, Sep 09, 2021 at 01:32:19PM +0100, Daniel P. Berrangé wrote: > This is obsolete since SeaBIOS 1.11.0 introduced native support for > sending messages to the serial console. The new support can be > activated using -machine graphics=off on x86 targets. > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Gerd Hoffmann
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
On 9/9/2021 10:47 PM, Michal Prívozník wrote:
> On 9/9/21 1:45 PM, Peng Liang wrote:
>> On 9/9/2021 7:01 PM, Michal Prívozník wrote:
>>> On 8/23/21 4:41 AM, Peng Liang wrote:
Signed-off-by: Peng Liang
---
src/libvirt_private.syms| 1 +
src/security/security_driver.h | 5 +
src/security/security_manager.c | 29 +
src/security/security_manager.h | 5 +
4 files changed, 40 insertions(+)
>>>
>>>
diff --git a/src/security/security_manager.c
b/src/security/security_manager.c
index 9906c1691d0f..b580704d3abf 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -476,6 +476,35 @@
virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
}
+/**
+ * virSecurityManagerUpdateImageLabel:
+ * @mgr: security manager object
+ * @vm: domain definition object
+ * @src: disk source definition to operate on
+ * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
+ *
+ * Update security label from @src according to @flags.
+ *
+ * Returns: 0 on success, -1 on error.
+ */
+int
+virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
+ virDomainDef *vm,
+ virStorageSource *src,
+ virSecurityDomainImageLabelFlags flags)
+{
+if (mgr->drv->domainUpdateSecurityImageLabel) {
+int ret;
+virObjectLock(mgr);
+ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src,
flags);
+virObjectUnlock(mgr);
+return ret;
+}
+
+return 0;
+}
+
+
>>>
>>> Is there a reason why this needs to be inside virSecurityManager? We
>>> already have virSecurityMoveRememberedLabel() that lives outside of it,
>>> in security_util.c and conceptually this function belongs there.
>>>
>>> Michal
>>>
>>> .
>>>
>> Maybe all security managers' labels need to be updated during migration,
>> so I add it here.
>
> Ah, you are correct. The timestamp XATTR is specific to secdriver so DAC
> and SELinux have their own timestamps. So your approach is in fact
> correct. For your v2 can you please also implement SELinux? I think it's
> going to be 1:1 copy of DAC code.
>
> Michal
>
> .
>
OK，I'll add and test it in v2.
Thanks for your reviewing!
Peng
Re: [PATCH v2 0/4] Add support for two i386 pm options which control acpi hotplug
On 8/19/21 6:50 AM, Ani Sinha wrote: Hi: I added some negative xml2argv tests as well as new xml2xml tests. In the process, I also fixed a bug where I had not added appropriate code to generate the output xml correctly. The patch series is sent again as v2. Kindly, please provide inputs and review them. [PATCH v2 1/4] pm/i386: add support for two options that controls [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug Hi, Sorry for not sending email about this earlier. We went over a few things in an IRC chat a couple weeks ago. I'll reiterate the questions I asked there, and fill in more info on what we are looking for in patches. * Usually when a new feature like this is supported, there will be several patches, divided something like this: 1) A patch that just adds the new QEMU capabilities flag(s) that will be used later to detect whether or not the selected QEMU binary supports the feature. (This might require updates to the sample QEMU capabilities test files, if the flag/option didn't happen to coincidentally already be in them. Instructions for regenerating the capabilities .replies files are in the comments of tests/qemucapabilitiestest.c.) 2) A patch adding the new XML to the RNG and to the XML parser in conf. This patch will also contain 2b) an addition to XML documentation in docs/formatdomain.rst that names & describes the purpose of the new elements/attributes and provides at least one example of their use (the commit log message should include an example too, to make searching for the commit easier), and 2c) parser/formatter tests added to qemuxml2xmltest.c, with the original XML in tests/qemuxml2argvdata, and output XML in qemuxml2xmlout (if the input and output files are identical, then the file in qemuxml2xmlout should be a symlink to the file in qemuxml2argvdata) 3) A patch that implements the backend of this new feature in the qemu driver by checking for its availability using the capabilities in patch 1, and using the data in the domain object now being parsed by patch 2 to add something to the qemu commandline. This patch should also include 3b) all additional test cases for qemuxml2argvtest. By careful planning, these new test cases will use the same source XML that was added in (2c). Note that *all* tests for new code are in the same patch as the new code itself. I like doing it this way because it ensures that the tests won't be forgotten/omitted on purpose when backporting to a different branch. 4) Oh, and just for abologna, a separate patch to add an entry to the NEWS.rst file for the next release :-) 0) (backing up a bit) In addition, the cover-letter (patch 0/n) should contain a *thorough* description of what each new XML element/attribute does, why it is desirable, and a link to the QEMU documentation (and/or patches as pushed into qemu) describing what they do in QEMU terms. I *think* that the 440fx-only option you added in these patches disables/enables hotplug of devices with a single systemwide flag (right?); I'm still uncertain what the other option does - apparently something about enabling the hotplug of a conventional PCI bridge? Or does it enable/disable hotplug of endpoint devices *into* conventional PCI bridges?). This information could be included directly in the cover letter, or the cover letter can point to the commit log message for patch 2 or 3 which would then have all the information. = About the "Why?" question above - many years ago someone decided that every feature added to QEMU *must* be supported directly in libvirt. This led to a large bloat of XML to support QEMU features that "seemed like a good idea at the time", but nobody ever uses (partly because it's unclear exactly how/when they should be used). In the more recent past, we've started asking "Is the maintenance burden of supporting this feature in libvirt really worthwhile? Is it usable? Who will actually use it, and for what?" (for a few years both QEMU and libvirt have been trying to get away from conventional PCI + 440fx, and concentrate our efforts on PCIe + Q35, so adding new functionality for conventional PCI + 440fx feels kind of like adding a new option to the IDE disk controller :-) I had questions on this topic for these new options, but realized that they all depended on my proper understanding of their function, and since I don't think I properly understand them yet, all my questions are potentially pointless, so I've removed them for now. Maybe it will all be clear once I've been properly informed. In the meantime, although not officially "supported" (I believe its use will taint the libvirt config) it's possible to add any random option not supported by libvirt to the QEMU commandline with libvirt's element, docum
[libvirt PATCH 1/2] conf: reformat virDomainDefCompatibleDevice for upcoming additional check
The next patch will add another check similar to the existing check
for a change in alias name. This patch reformats the code in
preparation so that the next patch's purpose will be clear.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cb9e7218ff..fefc527901 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28477,13 +28477,14 @@ virDomainDefCompatibleDevice(virDomainDef *def,
data.oldInfo = virDomainDeviceGetInfo(oldDev);
if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE &&
-live &&
-(data.newInfo && data.oldInfo &&
- data.newInfo->alias && data.oldInfo->alias &&
- STRNEQ(data.newInfo->alias, data.oldInfo->alias))) {
-virReportError(VIR_ERR_OPERATION_DENIED, "%s",
- _("changing device alias is not allowed"));
-return -1;
+live && data.newInfo && data.oldInfo) {
+
+if (data.newInfo->alias && data.oldInfo->alias &&
+STRNEQ(data.newInfo->alias, data.oldInfo->alias)) {
+virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+ _("changing device alias is not allowed"));
+return -1;
+}
}
if (!virDomainDefHasUSB(def) &&
--
2.31.1
[libvirt PATCH 2/2] conf: log error on attempts to modify ACPI index of active device
The ACPI index of a device in a running guest can't be modified, and
libvirt doesn't actually attempt to modify it, but it was possible for
a user to request such a modification, and libvirt wouldn't complain,
thus misleading the user into thinking that it had actually been changed.
Resolves: https://bugzilla.redhat.com/1998920
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fefc527901..7ff890d8b7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28485,6 +28485,12 @@ virDomainDefCompatibleDevice(virDomainDef *def,
_("changing device alias is not allowed"));
return -1;
}
+
+if (data.newInfo->acpiIndex != data.oldInfo->acpiIndex) {
+virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+ _("changing device 'acpi index' is not allowed"));
+return -1;
+}
}
if (!virDomainDefHasUSB(def) &&
--
2.31.1
[libvirt PATCH 0/2] conf: log error when attempting to live update device acpi index
This isn't possible (acpi index is set when devices are probed at guest startup) and libvirt wasn't even trying to communicate the change to the guest in any way, but instead of logging an error, we were just pretending the request had succeeded. Laine Stump (2): conf: reformat virDomainDefCompatibleDevice for upcoming additional check conf: log error on attempts to modify ACPI index of active device src/conf/domain_conf.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) -- 2.31.1
[RFC REPOST 2/7] cgroup: extract setting fibre channel appid into virCgroupSetFCAppid
Signed-off-by: Pavel Hrdina
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_cgroup.c | 17 +
src/util/vircgroup.c | 24
src/util/vircgroup.h | 3 +++
4 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d1f5dc2080..8bb349e7da 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1957,6 +1957,7 @@ virCgroupSetCpusetCpus;
virCgroupSetCpusetMemoryMigrate;
virCgroupSetCpusetMems;
virCgroupSetCpuShares;
+virCgroupSetFCAppid;
virCgroupSetFreezerState;
virCgroupSetMemory;
virCgroupSetMemoryHardLimit;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 6d4a82b3cd..9eec9db65c 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -908,27 +908,12 @@ static int
qemuSetupCgroupAppid(virDomainObj *vm)
{
qemuDomainObjPrivate *priv = vm->privateData;
-int inode = -1;
-const char *path = "/sys/class/fc/fc_udev_device/appid_store";
-g_autofree char *appid = NULL;
virDomainResourceDef *resource = vm->def->resource;
if (!resource || !resource->appid)
return 0;
-inode = virCgroupGetInode(priv->cgroup);
-if (inode < 0)
-return -1;
-
-appid = g_strdup_printf("%X:%s", inode, resource->appid);
-
-if (virFileWriteStr(path, appid, 0) < 0) {
-virReportSystemError(errno,
- _("Unable to write '%s' to '%s'"), appid, path);
-return -1;
-}
-
-return 0;
+return virCgroupSetFCAppid(priv->cgroup, resource->appid);
}
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 37b63a2e2d..ad0ee20862 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4012,3 +4012,27 @@ virCgroupGetCpuPeriodQuota(virCgroup *cgroup, unsigned
long long *period,
return 0;
}
+
+
+int
+virCgroupSetFCAppid(virCgroup *group,
+const char *appid)
+{
+int inode = -1;
+const char *path = "/sys/class/fc/fc_udev_device/appid_store";
+g_autofree char *vmid = NULL;
+
+inode = virCgroupGetInode(group);
+if (inode < 0)
+return -1;
+
+vmid = g_strdup_printf("%X:%s", inode, appid);
+
+if (virFileWriteStr(path, vmid, 0) < 0) {
+virReportSystemError(errno,
+ _("Unable to write '%s' to '%s'"), vmid, path);
+return -1;
+}
+
+return 0;
+}
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 690f09465c..a69b435b71 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -285,3 +285,6 @@ int virCgroupHasEmptyTasks(virCgroup *cgroup, int
controller);
bool virCgroupControllerAvailable(int controller);
int virCgroupGetInode(virCgroup *cgroup);
+
+int virCgroupSetFCAppid(virCgroup *group,
+const char *appid);
--
2.31.1
[RFC REPOST 7/7] tools: introduce virsh setappid command
Signed-off-by: Pavel Hrdina
---
docs/manpages/virsh.rst | 14 +
tools/virsh-domain.c| 65 +
2 files changed, 79 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 9561b3f59d..36fc94808d 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -4515,6 +4515,20 @@ Output the IP address and port number for the VNC
display. If the information
is not available the processes will provide an exit code of 1.
+setappid
+
+
+**Syntax:**
+
+::
+
+ setappid domain appid
+
+Set the Fibre Channel appid string for domain that is used to create VMID to
+tag the traffic. Accepts only printable characters and maximal length is 128
+characters. To remove the appid from VM don't pass any appid.
+
+
DEVICE COMMANDS
===
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e5bd1fdd75..07d9c7f954 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14070,6 +14070,65 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd)
}
+/*
+ * "setappid" command
+ */
+static const vshCmdInfo info_setappid[] = {
+{.name = "help",
+ .data = N_("Set domain Fibre Channel appid")
+},
+{.name = "desc",
+ .data = N_("Set the Fibre Channel appid string for domain that is "
+"used to create VMID to tag the traffic. Accepts only "
+"printable characters and maximal length is 128 characters. "
+"To remove the appid from VM don't pass any appid.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_setappid[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+VIRSH_COMMON_OPT_DOMAIN_LIVE,
+VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+{.name = "appid",
+ .type = VSH_OT_STRING,
+ .help = N_("User provided appid string"),
+},
+{.name = NULL}
+};
+
+static bool
+cmdSetAppid(vshControl *ctl, const vshCmd *cmd)
+{
+g_autoptr(virshDomain) dom = NULL;
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+bool current = vshCommandOptBool(cmd, "current");
+const char *appid = NULL;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptStringReq(ctl, cmd, "appid", &appid) < 0)
+return false;
+
+if (virDomainSetFibreChannelAppid(dom, (char *)appid, flags) < 0)
+return false;
+
+return true;
+}
+
+
const vshCmdDef domManagementCmds[] = {
{.name = "attach-device",
.handler = cmdAttachDevice,
@@ -14715,5 +14774,11 @@ const vshCmdDef domManagementCmds[] = {
.info = info_domdirtyrate_calc,
.flags = 0
},
+{.name = "setappid",
+ .handler = cmdSetAppid,
+ .opts = opts_setappid,
+ .info = info_setappid,
+ .flags = 0
+},
{.name = NULL}
};
--
2.31.1
[RFC REPOST 6/7] qemu: implement virDomainSetFibreChannelAppid API
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_driver.c | 75 ++
1 file changed, 75 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bd41ddbc3c..872dedeafe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20428,6 +20428,80 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom,
}
+static void
+qemuDomainUpdateFibreChannelAppid(virDomainDef *def,
+ const char *appid)
+{
+if (!def->resource) {
+def->resource = g_new0(virDomainResourceDef, 1);
+} else {
+g_free(def->resource->appid);
+}
+
+def->resource->appid = g_strdup(appid);
+}
+
+
+static int
+qemuDomainSetFibreChannelAppid(virDomainPtr dom,
+ const char *appid,
+ unsigned int flags)
+{
+virQEMUDriver *driver = dom->conn->privateData;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+virDomainObj *vm = NULL;
+virDomainDef *def = NULL;
+virDomainDef *persistentDef = NULL;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (appid && virDomainDefResourceAppidValidate(appid) < 0)
+return -1;
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainSetFibreChannelAppidEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+goto endjob;
+
+if (def) {
+qemuDomainObjPrivate *priv = vm->privateData;
+
+if (virCgroupSetFCAppid(priv->cgroup, appid) < 0)
+goto endjob;
+
+qemuDomainUpdateFibreChannelAppid(def, appid);
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+goto endjob;
+}
+
+if (persistentDef) {
+qemuDomainUpdateFibreChannelAppid(persistentDef, appid);
+
+if (virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) <
0)
+goto endjob;
+}
+
+ret = 0;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+
static virHypervisorDriver qemuHypervisorDriver = {
.name = QEMU_DRIVER_NAME,
.connectURIProbe = qemuConnectURIProbe,
@@ -20671,6 +20745,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainAuthorizedSSHKeysSet = qemuDomainAuthorizedSSHKeysSet, /* 6.10.0 */
.domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */
.domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */
+.domainSetFibreChannelAppid = qemuDomainSetFibreChannelAppid, /* 7.7.0 */
};
--
2.31.1
[RFC REPOST 4/7] src: introduce virDomainSetFibreChannelAppid API
Signed-off-by: Pavel Hrdina
---
include/libvirt/libvirt-domain.h | 4 +++
src/driver-hypervisor.h | 6 +
src/libvirt-domain.c | 44
src/libvirt_public.syms | 1 +
4 files changed, 55 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7ef8ac51e5..4f7b88ef61 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5153,4 +5153,8 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain,
int seconds,
unsigned int flags);
+int virDomainSetFibreChannelAppid(virDomainPtr domain,
+ const char *appid,
+ unsigned int flags);
+
#endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index d642af8a37..e6b1ceb3ce 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1412,6 +1412,11 @@ typedef int
typedef struct _virHypervisorDriver virHypervisorDriver;
+typedef int
+(*virDrvDomainSetFibreChannelAppid)(virDomainPtr domain,
+const char *appid,
+unsigned int flags);
+
/**
* _virHypervisorDriver:
*
@@ -1676,4 +1681,5 @@ struct _virHypervisorDriver {
virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet;
virDrvDomainGetMessages domainGetMessages;
virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc;
+virDrvDomainSetFibreChannelAppid domainSetFibreChannelAppid;
};
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index a8a386e839..f3e6854a39 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -13229,3 +13229,47 @@ virDomainStartDirtyRateCalc(virDomainPtr domain,
virDispatchError(conn);
return -1;
}
+
+
+/**
+ * virDomainSetFibreChannelAppid:
+ * @domain: a domain object
+ * @appid: user provided appid string
+ * @flags: extra flags
+ *
+ * Set the Fibre Channel APPID. Accepts only printable characters
+ * and maximal length is 128 characters. To remove the APPID use
+ * NULL as @appid value.
+ *
+ * Returns 0 in case of success, -1 otherwise.
+ */
+int
+virDomainSetFibreChannelAppid(virDomainPtr domain,
+ const char *appid,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "appid=%s, flags=0x%x", appid, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+
+virCheckReadOnlyGoto(conn->flags, error);
+
+if (conn->driver->domainSetFibreChannelAppid) {
+int ret;
+ret = conn->driver->domainSetFibreChannelAppid(domain, appid, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 3a5fa7cb09..912b421632 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -900,6 +900,7 @@ LIBVIRT_7.7.0 {
global:
virNWFilterDefineXMLFlags;
virNetworkDefineXMLFlags;
+virDomainSetFibreChannelAppid;
} LIBVIRT_7.3.0;
# define new API here using predicted next version number
--
2.31.1
[RFC REPOST 3/7] virCgroupSetFCAppid: properly handle when appid is NULL
With introduction of live changes of appid we should also support
removal of the appid from VM. This is done by writing empty appid part
to appid_store file.
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index ad0ee20862..9470ab061d 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4026,7 +4026,7 @@ virCgroupSetFCAppid(virCgroup *group,
if (inode < 0)
return -1;
-vmid = g_strdup_printf("%X:%s", inode, appid);
+vmid = g_strdup_printf("%X:%s", inode, appid ? appid : "");
if (virFileWriteStr(path, vmid, 0) < 0) {
virReportSystemError(errno,
--
2.31.1
[RFC REPOST 0/7] introduce support for live appid updates
Rebased on top of current master. I'm posting this as an RFC mainly because I'm not sure how to model the new API. This patches introduce a new naive API that will change only the APPID and nothing else. Currently there are no other known features related to Fibre Channel resources so this non-extendable API will be sufficient, however the appid lives in element in the XML where we currently have root cgroup partition. Even though changing the partition will not be supported and we don't know about anything else that could be placed here it doesn't mean it will not happen in the future. In that case we would have to add new API as well. So I'm wondering if we should create a more generic API that would take typed parameters as arguments: int virDomainSetResource(virDomainPtr domain, virTypedParameterPtr params, int nparams, unsigned int flags) Any ideas? Pavel Hrdina (7): conf: extract appid validation to virDomainDefResourceAppidValidate cgroup: extract setting fibre channel appid into virCgroupSetFCAppid virCgroupSetFCAppid: properly handle when appid is NULL src: introduce virDomainSetFibreChannelAppid API remote: add RPC support for the virDomainSetFibreChannelAppid API qemu: implement virDomainSetFibreChannelAppid API tools: introduce virsh setappid command docs/manpages/virsh.rst | 14 ++ include/libvirt/libvirt-domain.h | 4 ++ src/conf/domain_validate.c | 42 ++ src/conf/domain_validate.h | 2 + src/driver-hypervisor.h | 6 +++ src/libvirt-domain.c | 44 +++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 1 + src/qemu/qemu_cgroup.c | 17 +--- src/qemu/qemu_driver.c | 75 src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +- src/remote_protocol-structs | 6 +++ src/util/vircgroup.c | 24 ++ src/util/vircgroup.h | 3 ++ tools/virsh-domain.c | 65 +++ 16 files changed, 286 insertions(+), 34 deletions(-) -- 2.31.1
[RFC REPOST 5/7] remote: add RPC support for the virDomainSetFibreChannelAppid API
Signed-off-by: Pavel Hrdina
---
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 14 +-
src/remote_protocol-structs | 6 ++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b64a86af63..55a0b79f95 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8539,6 +8539,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainAuthorizedSSHKeysSet = remoteDomainAuthorizedSSHKeysSet, /* 6.10.0
*/
.domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */
.domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */
+.domainSetFibreChannelAppid = remoteDomainSetFibreChannelAppid, /* 7.7.0 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index df1b126b0c..0a64504d45 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3854,6 +3854,12 @@ struct remote_domain_start_dirty_rate_calc_args {
unsigned int flags;
};
+struct remote_domain_set_fibre_channel_appid_args {
+remote_nonnull_domain dom;
+remote_string appid;
+unsigned int flags;
+};
+
/*- Protocol. -*/
@@ -6818,5 +6824,11 @@ enum remote_procedure {
* @acl: network:write
* @acl: network:save
*/
-REMOTE_PROC_NETWORK_DEFINE_XML_FLAGS = 432
+REMOTE_PROC_NETWORK_DEFINE_XML_FLAGS = 432,
+
+/**
+ * @generate: both
+ * @acl: domain:write
+ */
+REMOTE_PROC_DOMAIN_SET_FIBRE_CHANNEL_APPID = 433
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index dad83361fa..731cad112c 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -3206,6 +3206,11 @@ struct remote_domain_start_dirty_rate_calc_args {
intseconds;
u_int flags;
};
+struct remote_domain_set_fibre_channel_appid_args {
+remote_nonnull_domain dom;
+remote_string appid;
+u_int flags;
+};
enum remote_procedure {
REMOTE_PROC_CONNECT_OPEN = 1,
REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3639,4 +3644,5 @@ enum remote_procedure {
REMOTE_PROC_NODE_DEVICE_CREATE = 430,
REMOTE_PROC_NWFILTER_DEFINE_XML_FLAGS = 431,
REMOTE_PROC_NETWORK_DEFINE_XML_FLAGS = 432,
+REMOTE_PROC_DOMAIN_SET_FIBRE_CHANNEL_APPID = 433,
};
--
2.31.1
[RFC REPOST 1/7] conf: extract appid validation to virDomainDefResourceAppidValidate
This will be needed by future patches adding appid API to allow changing
it for running VMs.
Signed-off-by: Pavel Hrdina
---
src/conf/domain_validate.c | 42 +++---
src/conf/domain_validate.h | 2 ++
src/libvirt_private.syms | 1 +
3 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 1bc62c364d..3ab864bbeb 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -58,29 +58,37 @@ virDomainDefBootValidate(const virDomainDef *def)
#define APPID_LEN_MIN 1
#define APPID_LEN_MAX 128
+int
+virDomainDefResourceAppidValidate(const char *appid)
+{
+int len;
+
+if (!virStringIsPrintable(appid)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Fibre Channel 'appid' is not a printable string"));
+return -1;
+}
+
+len = strlen(appid);
+if (len < APPID_LEN_MIN || len > APPID_LEN_MAX) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("Fibre Channel 'appid' string length must be between
[%d, %d]"),
+ APPID_LEN_MIN, APPID_LEN_MAX);
+return -1;
+}
+
+return 0;
+}
+
+
static int
virDomainDefResourceValidate(const virDomainDef *def)
{
if (!def->resource)
return 0;
-if (def->resource->appid) {
-int len;
-
-if (!virStringIsPrintable(def->resource->appid)) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Fibre Channel 'appid' is not a printable
string"));
-return -1;
-}
-
-len = strlen(def->resource->appid);
-if (len < APPID_LEN_MIN || len > APPID_LEN_MAX) {
-virReportError(VIR_ERR_INVALID_ARG,
- _("Fibre Channel 'appid' string length must be
between [%d, %d]"),
- APPID_LEN_MIN, APPID_LEN_MAX);
-return -1;
-}
-}
+if (def->resource->appid)
+return virDomainDefResourceAppidValidate(def->resource->appid);
return 0;
}
diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h
index 430d61fd3c..02fc16b17d 100644
--- a/src/conf/domain_validate.h
+++ b/src/conf/domain_validate.h
@@ -42,3 +42,5 @@ int virDomainDeviceDefValidate(const virDomainDeviceDef *dev,
int virDomainDiskDefValidateSource(const virStorageSource *src);
int virDomainDiskDefSourceLUNValidate(const virStorageSource *src);
+
+int virDomainDefResourceAppidValidate(const char *appid);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2778fe7f8f..d1f5dc2080 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -765,6 +765,7 @@ virDomainConfVMNWFilterTeardown;
# conf/domain_validate.h
virDomainActualNetDefValidate;
+virDomainDefResourceAppidValidate;
virDomainDefValidate;
virDomainDeviceValidateAliasForHotplug;
virDomainDiskDefSourceLUNValidate;
--
2.31.1
[libvirt PATCH] docs: virtiofs: remove extra slash
Reported-by: Richard W.M. Jones Signed-off-by: Ján Tomko --- Pushed as trivial. docs/kbase/virtiofs.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index b89de0c57a..9123fc2316 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -81,7 +81,7 @@ control and need to be set by the application running virtiofsd. :: - + -- 2.31.1
[libvirt PATCH] docs: virtiofs: provide more context for elements
Suggested-by: Richard W.M. Jones Signed-off-by: Ján Tomko --- Pushed as trivial. docs/kbase/virtiofs.rst | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index d728a1358c..b89de0c57a 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -62,11 +62,14 @@ More optional elements can be specified :: - - - - - + + +... + + + + + Externally-launched virtiofsd = -- 2.31.1
Re: [libvirt PATCH 1/2] docs: virtiofs: move legacy docs to the bottom
On a Thursday in 2021, Richard W.M. Jones wrote: On Thu, Sep 09, 2021 at 03:58:28PM +0100, Stefan Hajnoczi wrote: A number of legacy issues make the virtiofs kbase article hard to understand. Most users don't need to configure NUMA or a memory backend other than memfd. Move that information to the bottom of the article so the recommended syntax is most prominent. Suggested-by: Richard W.M. Jones Signed-off-by: Stefan Hajnoczi This is much better. There are a couple of minor points below but in general: Both of those are in pre-existing text that only shows up as '+' lines due to the move. I pushed the fixes as separate trivial patches. Thank you. Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 2/2] docs: virtiofs: use the preferred virtiofs spelling
On Thu, Sep 09, 2021 at 05:33:43PM +0200, Ján Tomko wrote: > On a Thursday in 2021, Stefan Hajnoczi wrote: > > The virtiofs project started off using "virtio-fs" but later switched to > > the "virtiofs" spelling because it matches the spelling of the mount -t > > virtiofs command-line. Update the kbase article with the new spelling so > > it matches the virtiofs website. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > docs/kbase/virtiofs.rst | 12 ++-- > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > There are two more occurrences on pages linking to the kbase article. > > With the following diff squashed in: > Reviewed-by: Ján Tomko Thanks for spotting this! This looks like it can be squashed when merging so I won't resend unless there are other items that need to be addressed. Stefan > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index 479a3acfbb..ad3b4ea92c 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -3305,7 +3305,7 @@ A directory on the host that can be accessed directly > from the guest. >pages touched during a guest file write operation :since:`(since > 0.9.10)` >. :since:`Since 6.2.0` , ``type='virtiofs'`` is also supported. Using >virtiofs requires setting up shared memory, see the guide: > - `Virtio-FS `__ > + `Virtiofs `__ > ``template`` >OpenVZ filesystem template. Only used by OpenVZ driver. > ``file`` > diff --git a/docs/kbase/index.rst b/docs/kbase/index.rst > index 372042886d..351ea2c93b 100644 > --- a/docs/kbase/index.rst > +++ b/docs/kbase/index.rst > @@ -12,7 +12,7 @@ Usage > Explanation of how disk backing chain specification impacts libvirt's > behaviour and basic troubleshooting steps of disk problems. > > -`Virtio-FS `__ > +`Virtiofs `__ > Share a filesystem between the guest and the host > > `Security with QEMU passthrough `__ > signature.asc Description: PGP signature
Re: [libvirt PATCH 1/2] docs: virtiofs: move legacy docs to the bottom
On Thu, Sep 09, 2021 at 03:58:28PM +0100, Stefan Hajnoczi wrote: > A number of legacy issues make the virtiofs kbase article hard to > understand. Most users don't need to configure NUMA or a memory backend > other than memfd. Move that information to the bottom of the article so > the recommended syntax is most prominent. > > Suggested-by: Richard W.M. Jones > Signed-off-by: Stefan Hajnoczi This is much better. There are a couple of minor points below but in general: Reviewed-by: Richard W.M. Jones > +Optional parameters > +=== > + > +More optional elements can be specified > + > +:: > + > + > + > + > + > + I always think with XML it's better to be completely clear about the context around the element so that users know where to put these extra elements. For this reason I'd probably write this: + + +... + + + + + > +Externally-launched virtiofsd > += > + > +Libvirtd can also connect the ``vhost-user-fs`` device to a ``virtiofsd`` > +daemon launched outside of libvirtd. In that case socket permissions, > +the mount tag and all the virtiofsd options are out of libvirtd's > +control and need to be set by the application running virtiofsd. > + > +:: > + > + Is there an extra '/' above ^^ ? I see the same mistake is present in the current page. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [libvirt PATCH 2/2] docs: virtiofs: use the preferred virtiofs spelling
On a Thursday in 2021, Stefan Hajnoczi wrote: The virtiofs project started off using "virtio-fs" but later switched to the "virtiofs" spelling because it matches the spelling of the mount -t virtiofs command-line. Update the kbase article with the new spelling so it matches the virtiofs website. Signed-off-by: Stefan Hajnoczi --- docs/kbase/virtiofs.rst | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) There are two more occurrences on pages linking to the kbase article. With the following diff squashed in: Reviewed-by: Ján Tomko Jano diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 479a3acfbb..ad3b4ea92c 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3305,7 +3305,7 @@ A directory on the host that can be accessed directly from the guest. pages touched during a guest file write operation :since:`(since 0.9.10)` . :since:`Since 6.2.0` , ``type='virtiofs'`` is also supported. Using virtiofs requires setting up shared memory, see the guide: - `Virtio-FS `__ + `Virtiofs `__ ``template`` OpenVZ filesystem template. Only used by OpenVZ driver. ``file`` diff --git a/docs/kbase/index.rst b/docs/kbase/index.rst index 372042886d..351ea2c93b 100644 --- a/docs/kbase/index.rst +++ b/docs/kbase/index.rst @@ -12,7 +12,7 @@ Usage Explanation of how disk backing chain specification impacts libvirt's behaviour and basic troubleshooting steps of disk problems. -`Virtio-FS `__ +`Virtiofs `__ Share a filesystem between the guest and the host `Security with QEMU passthrough `__ signature.asc Description: PGP signature
Re: [libvirt PATCH 1/2] docs: virtiofs: move legacy docs to the bottom
On a Thursday in 2021, Stefan Hajnoczi wrote: A number of legacy issues make the virtiofs kbase article hard to understand. Most users don't need to configure NUMA or a memory backend other than memfd. Move that information to the bottom of the article so the recommended syntax is most prominent. Suggested-by: Richard W.M. Jones Signed-off-by: Stefan Hajnoczi --- docs/kbase/virtiofs.rst | 171 +--- 1 file changed, 91 insertions(+), 80 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 23/24] tests: vir: remove pointless labels
On 9/9/21 10:28 AM, Ján Tomko wrote:
On a Monday in 2021, Laine Stump wrote:
On 9/4/21 4:44 PM, Ján Tomko wrote:
@@ -319,9 +308,9 @@ testFileIsSharedFSType(const void *opaque
G_GNUC_UNUSED)
return EXIT_AM_SKIP;
#else
const struct testFileIsSharedFSType *data = opaque;
+ int ret = -1;
g_autofree char *mtabFile = NULL;
bool actual;
- int ret = -1;
unrelated (well, only peripherally related) code movement.
[...]
@@ -199,15 +195,15 @@ testVirPCIDeviceReattach(const void *opaque
G_GNUC_UNUSED)
for (i = 0; i < nDev; i++) {
if (virPCIDeviceReattach(dev[i], activeDevs, inactiveDevs) < 0)
- goto cleanup;
+ return -1;
CHECK_LIST_COUNT(activeDevs, 0);
CHECK_LIST_COUNT(inactiveDevs, nDev - i - 1);
}
- ret = 0;
+ return 0;
cleanup:
- return ret;
+ return -1;
}
Oops. You forgot one! cleanup is no longer referenced, but you didn't
remove it (or the dead code "return -1;" following it)
'cleanup' is still referenced inside the 'CHECK_LIST_COUNT' macro,
which is also used by testVirPCIDeviceReset and testVirPCIDeviceDetach.
Ewww, ... er, I mean "Oh. Interesting." Well at least that explains why
it still compiled, so I don't have to lose sleep over that any more :-)
On second thought, mixing returns with cleanups does not seem like
a good idea and I will drop this partial conversion before pushing.
Jano
[libvirt PATCH 2/2] docs: virtiofs: use the preferred virtiofs spelling
The virtiofs project started off using "virtio-fs" but later switched to the "virtiofs" spelling because it matches the spelling of the mount -t virtiofs command-line. Update the kbase article with the new spelling so it matches the virtiofs website. Signed-off-by: Stefan Hajnoczi --- docs/kbase/virtiofs.rst | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index 75e740bf96..d728a1358c 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -1,13 +1,13 @@ - -Sharing files with Virtio-FS - +=== +Sharing files with Virtiofs +=== .. contents:: -Virtio-FS -= +Virtiofs + -Virtio-FS is a shared file system that lets virtual machines access +Virtiofs is a shared file system that lets virtual machines access a directory tree on the host. Unlike existing approaches, it is designed to offer local file system semantics and performance. -- 2.31.1
[libvirt PATCH 1/2] docs: virtiofs: move legacy docs to the bottom
A number of legacy issues make the virtiofs kbase article hard to understand. Most users don't need to configure NUMA or a memory backend other than memfd. Move that information to the bottom of the article so the recommended syntax is most prominent. Suggested-by: Richard W.M. Jones Signed-off-by: Stefan Hajnoczi --- docs/kbase/virtiofs.rst | 171 +--- 1 file changed, 91 insertions(+), 80 deletions(-) diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index 6ba7299a72..75e740bf96 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -13,8 +13,82 @@ is designed to offer local file system semantics and performance. See https://virtio-fs.gitlab.io/ -Host setup -== +Sharing a host directory with a guest += + +#. Add the following domain XML elements to share the host directory `/path` + with the guest + + :: + + + ... + + + + + ... + + ... + + + + + + ... + + + + Don't forget the elements. They are necessary for the + vhost-user connection with the ``virtiofsd`` daemon. + + Note that despite its name, the ``target dir`` is an arbitrary string called + a mount tag that is used inside the guest to identify the shared file system + to be mounted. It does not have to correspond to the desired mount point in the + guest. + +#. Boot the guest and mount the filesystem + + :: + + guest# mount -t virtiofs mount_tag /mnt/mount/path + + Note: this requires virtiofs support in the guest kernel (Linux v5.4 or later) + +Optional parameters +=== + +More optional elements can be specified + +:: + + + + + + + +Externally-launched virtiofsd += + +Libvirtd can also connect the ``vhost-user-fs`` device to a ``virtiofsd`` +daemon launched outside of libvirtd. In that case socket permissions, +the mount tag and all the virtiofsd options are out of libvirtd's +control and need to be set by the application running virtiofsd. + +:: + + + + + + + +Other options for vhost-user memory setup += + +The following information is necessary if you are using older versions of QEMU +and libvirt or have special memory backend requirements. Almost all virtio devices (all that use virtqueues) require access to at least certain portions of guest RAM (possibly policed by DMA). In @@ -29,34 +103,31 @@ NUMA. As of QEMU 5.0.0 and libvirt 6.9.0, it is possible to specify the memory backend without NUMA (using the so called memobject interface). -One of the following: +#. Set up the memory backend -* Use memfd memory + * Use memfd memory - No host setup is required when using the Linux memfd memory backend. + No host setup is required when using the Linux memfd memory backend. -* Use file-backed memory + * Use file-backed memory - Configure the directory where the files backing the memory will be stored - with the ``memory_backing_dir`` option in ``/etc/libvirt/qemu.conf`` + Configure the directory where the files backing the memory will be stored + with the ``memory_backing_dir`` option in ``/etc/libvirt/qemu.conf`` - :: + :: -# This directory is used for memoryBacking source if configured as file. -# NOTE: big files will be stored here -memory_backing_dir = "/dev/shm/" + # This directory is used for memoryBacking source if configured as file. + # NOTE: big files will be stored here + memory_backing_dir = "/dev/shm/" -* Use hugepage-backed memory + * Use hugepage-backed memory - Make sure there are enough huge pages allocated for the requested guest memory. - For example, for one guest with 2 GiB of RAM backed by 2 MiB hugepages: + Make sure there are enough huge pages allocated for the requested guest memory. + For example, for one guest with 2 GiB of RAM backed by 2 MiB hugepages: - :: + :: - # virsh allocpages 2M 1024 - -Guest setup -=== + # virsh allocpages 2M 1024 #. Specify the NUMA topology (this step is only required for the NUMA case) @@ -122,63 +193,3 @@ Guest setup ... - -#. Add the ``vhost-user-fs`` QEMU device via the ``filesystem`` element - - :: - - -... - - ... - - - - - - ... - - - - Note that despite its name, the ``target dir`` is actually a mount tag and does - not have to correspond to the desired mount point in the guest. - - So far, ``passthrough`` is the only supported access mode and it requires - running the ``virtiofsd`` daemon as root. - -#. Boot the guest and mount the filesystem - - :: - - guest# mount -t virtiofs mount_tag /mnt/mount/path - - Note:
[libvirt PATCH 0/2] docs: virtiofs: move legacy docs to the bottom
The virtiofs kbase article includes a lot of information that is only relevant to old versions of QEMU and libvirt. Setting up virtiofs can seem intimidating but it's actually easier than the article lets on. Move the legacy information out of the way. Stefan Hajnoczi (2): docs: virtiofs: move legacy docs to the bottom docs: virtiofs: use the preferred virtiofs spelling docs/kbase/virtiofs.rst | 183 +--- 1 file changed, 97 insertions(+), 86 deletions(-) -- 2.31.1
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
On 9/9/21 1:45 PM, Peng Liang wrote:
> On 9/9/2021 7:01 PM, Michal Prívozník wrote:
>> On 8/23/21 4:41 AM, Peng Liang wrote:
>>> Signed-off-by: Peng Liang
>>> ---
>>> src/libvirt_private.syms| 1 +
>>> src/security/security_driver.h | 5 +
>>> src/security/security_manager.c | 29 +
>>> src/security/security_manager.h | 5 +
>>> 4 files changed, 40 insertions(+)
>>>
>>
>>
>>> diff --git a/src/security/security_manager.c
>>> b/src/security/security_manager.c
>>> index 9906c1691d0f..b580704d3abf 100644
>>> --- a/src/security/security_manager.c
>>> +++ b/src/security/security_manager.c
>>> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager
>>> *mgr,
>>> }
>>>
>>>
>>> +/**
>>> + * virSecurityManagerUpdateImageLabel:
>>> + * @mgr: security manager object
>>> + * @vm: domain definition object
>>> + * @src: disk source definition to operate on
>>> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
>>> + *
>>> + * Update security label from @src according to @flags.
>>> + *
>>> + * Returns: 0 on success, -1 on error.
>>> + */
>>> +int
>>> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
>>> + virDomainDef *vm,
>>> + virStorageSource *src,
>>> + virSecurityDomainImageLabelFlags flags)
>>> +{
>>> +if (mgr->drv->domainUpdateSecurityImageLabel) {
>>> +int ret;
>>> +virObjectLock(mgr);
>>> +ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src,
>>> flags);
>>> +virObjectUnlock(mgr);
>>> +return ret;
>>> +}
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +
>>
>> Is there a reason why this needs to be inside virSecurityManager? We
>> already have virSecurityMoveRememberedLabel() that lives outside of it,
>> in security_util.c and conceptually this function belongs there.
>>
>> Michal
>>
>> .
>>
> Maybe all security managers' labels need to be updated during migration,
> so I add it here.
Ah, you are correct. The timestamp XATTR is specific to secdriver so DAC
and SELinux have their own timestamps. So your approach is in fact
correct. For your v2 can you please also implement SELinux? I think it's
going to be 1:1 copy of DAC code.
Michal
Re: [libvirt PATCH 23/24] tests: vir: remove pointless labels
On a Monday in 2021, Laine Stump wrote:
On 9/4/21 4:44 PM, Ján Tomko wrote:
@@ -319,9 +308,9 @@ testFileIsSharedFSType(const void *opaque G_GNUC_UNUSED)
return EXIT_AM_SKIP;
#else
const struct testFileIsSharedFSType *data = opaque;
+int ret = -1;
g_autofree char *mtabFile = NULL;
bool actual;
-int ret = -1;
unrelated (well, only peripherally related) code movement.
[...]
@@ -199,15 +195,15 @@ testVirPCIDeviceReattach(const void *opaque G_GNUC_UNUSED)
for (i = 0; i < nDev; i++) {
if (virPCIDeviceReattach(dev[i], activeDevs, inactiveDevs) < 0)
-goto cleanup;
+return -1;
CHECK_LIST_COUNT(activeDevs, 0);
CHECK_LIST_COUNT(inactiveDevs, nDev - i - 1);
}
-ret = 0;
+return 0;
cleanup:
-return ret;
+return -1;
}
Oops. You forgot one! cleanup is no longer referenced, but you didn't
remove it (or the dead code "return -1;" following it)
'cleanup' is still referenced inside the 'CHECK_LIST_COUNT' macro,
which is also used by testVirPCIDeviceReset and testVirPCIDeviceDetach.
On second thought, mixing returns with cleanups does not seem like
a good idea and I will drop this partial conversion before pushing.
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH v3 0/7] Enable autostarting mediated devices
On 8/21/21 12:29 AM, Jonathon Jongsma wrote: > This first version of this series was reviewed quite a while ago and all > patches were ACKed except the second one. I posted a second series with > changes > noted below but it was never ACKed and I dropped the ball for a little while. > > Subsequently there were questions about whether physical devices (e.g. pci, > usb, etc) should return 'true' or 'false' for the > GetAutostart()/IsPersistent() > calls. I had initially marked them as persistent=true and autostart=true > because they superficially act a bit like persistently-defined devices. But > Boris convinced me that this is not a very accurate classification since if > the > physical device is unplugged, there will be no record of it left behind. So > I've switched all non-mdev devices to be persistent=false and autostart=false. > This is also imperfect, but it seems relatively harmless. Comments welcome. > > A reminder of what is included in this series: > - new API consistent with existing libvirt objects: > - virNodeDeviceGetAutostart() > - virNodeDeviceSetAutostart() > - virNodeDeviceIsPersistent() > - virNodeDeviceIsActive > - new virsh commands > - nodedev-info > - nodedev-autostart > > Changes in version 2: > - Parse the autostart property from mdevctl output. > > Changes in version 3: > - switch physical devices to autostart=false, persistent=false > - rebase to upstream > - update version numbers for new API, etc > - fix accidental copy-paste error in virsh command descriptions > > Jonathon Jongsma (7): > api: add virNodeDevice(Get|Set)Autostart() > nodedev: implement virNodeDevice(Get|Set)Autostart() > nodedev: Add tests for mdevctl autostart command > virsh: add nodedev-autostart > api: add virNodeDeviceIsPersistent()/IsActive() > nodedev: Implement virNodeDeviceIsPersistent()/IsActive() > virsh: add nodedev-info > > docs/manpages/virsh.rst | 27 +++ > include/libvirt/libvirt-nodedev.h | 10 + > src/conf/node_device_conf.h | 1 + > src/conf/virnodedeviceobj.c | 16 ++ > src/conf/virnodedeviceobj.h | 6 + > src/driver-nodedev.h | 18 ++ > src/libvirt-nodedev.c | 141 + > src/libvirt_private.syms | 2 + > src/libvirt_public.syms | 4 + > src/node_device/node_device_driver.c | 189 +- > src/node_device/node_device_driver.h | 19 ++ > src/node_device/node_device_udev.c| 22 +- > src/remote/remote_driver.c| 6 +- > src/remote/remote_protocol.x | 60 +- > src/remote_protocol-structs | 26 +++ > .../nodedevmdevctldata/mdevctl-autostart.argv | 8 + > tests/nodedevmdevctltest.c| 55 + > tools/virsh-nodedev.c | 139 + > 18 files changed, 740 insertions(+), 9 deletions(-) > create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv > Sorry for the delay. Reviewed-by: Michal Privoznik Michal
Re: [PATCH] virsh: Display vhostuser socket path in domblklist
On 9/3/21 10:54 AM, dinglimin wrote:
> The domblklist command is designed to show a brief information about the
> blocks of a domain.
> One piece of information that is shows is "Target "and "Source".
> Before the modification, the Vhost disk of SPDK is displayed as "-".
> After the modification, the socket associated with it can be displayed
>
> Signed-off-by: dinglimin
> ---
> tools/virsh-domain-monitor.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index f7cf82acdf..eb3e0ef11a 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -644,7 +644,8 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
> "|./source/@dev"
> "|./source/@dir"
> "|./source/@name"
> -"|./source/@volume)", ctxt);
> +"|./source/@volume"
> +"|./source/@path)", ctxt);
> }
>
> if (details) {
>
Reviewed-by: Michal Privoznik
and pushed. Congratulations on your first libvirt contribution!
Michal
[PATCH] hw/misc: deprecate the 'sga' device
This is obsolete since SeaBIOS 1.11.0 introduced native support for
sending messages to the serial console. The new support can be
activated using -machine graphics=off on x86 targets.
Signed-off-by: Daniel P. Berrangé
---
docs/about/deprecated.rst | 10 ++
hw/misc/sga.c | 2 ++
2 files changed, 12 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 9ee355ec0b..cafca05826 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -280,6 +280,16 @@ full SCSI support. Use virtio-scsi instead when SCSI
passthrough is required.
Note this also applies to ``-device virtio-blk-pci,scsi=on|off``, which is an
alias.
+``-device sga`` (since 6.2)
+^^^
+
+The ``sga`` device loads an option ROM for x86 targets which enables
+SeaBIOS to send messages to the serial console. SeaBIOS 1.11.0 onwards
+contains native support for this feature and thus use of the option
+ROM approach is obsolete. The native SeaBIOS support can be activated
+by using ``-machine graphics=off``.
+
+
Block device options
diff --git a/hw/misc/sga.c b/hw/misc/sga.c
index 4dbe6d78f9..1d04672b01 100644
--- a/hw/misc/sga.c
+++ b/hw/misc/sga.c
@@ -30,6 +30,7 @@
#include "hw/loader.h"
#include "qemu/module.h"
#include "qom/object.h"
+#include "qemu/error-report.h"
#define SGABIOS_FILENAME "sgabios.bin"
@@ -42,6 +43,7 @@ struct ISASGAState {
static void sga_realizefn(DeviceState *dev, Error **errp)
{
+warn_report("-device sga is deprecated, use -machine graphics=off");
rom_add_vga(SGABIOS_FILENAME);
}
--
2.31.1
Re: [libvirt PATCH 0/4] qemu: replace -device sga with -M graphics=off
On a Thursday in 2021, Daniel P. Berrangé wrote: SeaBIOS >= 1.11 / QEMU >= 2.11 no longer requires the 'sga' device for serial console output from the BIOS. It can be done directly with graphics=off machine option. This appears to be live migration compatible, despite changing the number of option ROMS loaded, though I need a little more testing to fully confirm this. Daniel P. Berrangé (4): qemu: prevent use of on non-x86 arches qemu: tweak error message to be more general purpose qemu: switch to use -M graphics=off instead of -device sga qemu: stop probing for '-device sga' support src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 25 --- src/qemu/qemu_validate.c | 15 --- .../caps_2.11.0.x86_64.xml| 1 - .../caps_2.12.0.x86_64.xml| 1 - .../caps_3.0.0.x86_64.xml | 1 - .../caps_3.1.0.x86_64.xml | 1 - .../caps_4.0.0.x86_64.xml | 1 - .../caps_4.1.0.x86_64.xml | 1 - .../caps_4.2.0.x86_64.xml | 1 - .../caps_5.0.0.x86_64.xml | 1 - .../caps_5.1.0.x86_64.xml | 1 - .../caps_5.2.0.x86_64.xml | 1 - .../caps_6.0.0.x86_64.xml | 1 - .../caps_6.1.0.x86_64.xml | 1 - tests/qemuxml2argvdata/bios.args | 3 +-- tests/qemuxml2argvtest.c | 3 +-- 18 files changed, 25 insertions(+), 38 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 1/4] qemu: prevent use of on non-x86 arches
On a Thursday in 2021, Daniel P. Berrangé wrote:
The config results in use of the '-device sga'
QEMU options. This in turn causes QEMU go load the sgabios.bin option
ROM, which contains x86 machine code. This cannot work on non-x86
arches, thus we should block the bad config.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_validate.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index d8f39b6bdd..3789361b57 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1194,6 +1194,14 @@ qemuValidateDomainDef(const virDomainDef *def,
/* Serial graphics adapter */
if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) {
+/* -device sga is only sane on x86, since the option ROM it
+ * loads contains x86 machine code.
+ */
QEMU_CAPS_SGA is only set on x86 QEMUs, so we have already prevented this
config in the past.
But in context of patch 3/4, adding this check makes sense.
+if (!ARCH_IS_X86(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("BIOS serial console only supported on x86
architectures"));
+return -1;
+}
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("qemu does not support SGA"));
--
2.31.1
signature.asc
Description: PGP signature
Re: [PATCH 3/3] qemu: Add support for virtio device option paeg-per-vq
On 9/6/21 4:06 PM, Han Han wrote:
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1925363
>
> Signed-off-by: Han Han
> ---
> src/qemu/qemu_command.c | 4 ++
> src/qemu/qemu_hotplug.c | 3 +-
> src/qemu/qemu_validate.c | 8
> .../virtio-options-controller-page_per_vq.err | 1 +
> ...-controller-page_per_vq.x86_64-latest.args | 37 ++
> .../virtio-options-controller-page_per_vq.xml | 38 ++
> .../virtio-options-disk-page_per_vq.err | 1 +
> ...ptions-disk-page_per_vq.x86_64-latest.args | 39 +++
> .../virtio-options-disk-page_per_vq.xml | 34
> .../virtio-options-fs-page_per_vq.err | 1 +
> ...-options-fs-page_per_vq.x86_64-latest.args | 37 ++
> .../virtio-options-fs-page_per_vq.xml | 34
> .../virtio-options-input-page_per_vq.err | 1 +
> ...tions-input-page_per_vq.x86_64-latest.args | 35 +
> .../virtio-options-input-page_per_vq.xml | 30 ++
> .../virtio-options-memballoon-page_per_vq.err | 1 +
> ...-memballoon-page_per_vq.x86_64-latest.args | 33
> .../virtio-options-memballoon-page_per_vq.xml | 23 +++
> .../virtio-options-net-page_per_vq.err| 1 +
> ...options-net-page_per_vq.x86_64-latest.args | 37 ++
> .../virtio-options-net-page_per_vq.xml| 34
> .../virtio-options-rng-page_per_vq.err| 1 +
> ...options-rng-page_per_vq.x86_64-latest.args | 37 ++
> .../virtio-options-rng-page_per_vq.xml| 32 +++
> .../virtio-options-video-page_per_vq.err | 1 +
> ...tions-video-page_per_vq.x86_64-latest.args | 37 ++
> .../virtio-options-video-page_per_vq.xml | 36 +
> .../virtio-options.x86_64-latest.args | 26 ++---
> tests/qemuxml2argvdata/virtio-options.xml | 26 ++---
> tests/qemuxml2argvtest.c | 22 +++
> 30 files changed, 623 insertions(+), 27 deletions(-)
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-controller-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-controller-page_per_vq.x86_64-latest.args
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-controller-page_per_vq.xml
> create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-disk-page_per_vq.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-page_per_vq.xml
> create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-fs-page_per_vq.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-page_per_vq.xml
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-input-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-input-page_per_vq.x86_64-latest.args
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-input-page_per_vq.xml
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-memballoon-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-memballoon-page_per_vq.x86_64-latest.args
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-memballoon-page_per_vq.xml
> create mode 100644 tests/qemuxml2argvdata/virtio-options-net-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-net-page_per_vq.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/virtio-options-net-page_per_vq.xml
> create mode 100644 tests/qemuxml2argvdata/virtio-options-rng-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-rng-page_per_vq.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/virtio-options-rng-page_per_vq.xml
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-video-page_per_vq.err
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-video-page_per_vq.x86_64-latest.args
> create mode 100644
> tests/qemuxml2argvdata/virtio-options-video-page_per_vq.xml
Wow, that's a lot of test cases. Do we need all of them?
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index b230314f7f..549f11dbe8 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -645,6 +645,10 @@ qemuBuildVirtioOptionsStr(virBuffer *buf,
> virBufferAsprintf(buf, ",packed=%s",
>virTristateSwitchTypeToString(virtio->packed));
> }
> +if (virtio->page_per_vq != VIR_TRISTATE_SWITCH_ABSENT) {
> +virBufferAsprintf(buf, ",page-per-vq=%s",
> +
> virTristateSwitchTypeToString(virtio->page_per_vq));
> +}
> }
>
> static int
> diff --git a/src/qemu/qemu_hotplug.c
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
On 9/9/2021 7:01 PM, Michal Prívozník wrote:
> On 8/23/21 4:41 AM, Peng Liang wrote:
>> Signed-off-by: Peng Liang
>> ---
>> src/libvirt_private.syms| 1 +
>> src/security/security_driver.h | 5 +
>> src/security/security_manager.c | 29 +
>> src/security/security_manager.h | 5 +
>> 4 files changed, 40 insertions(+)
>>
>
>
>> diff --git a/src/security/security_manager.c
>> b/src/security/security_manager.c
>> index 9906c1691d0f..b580704d3abf 100644
>> --- a/src/security/security_manager.c
>> +++ b/src/security/security_manager.c
>> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager
>> *mgr,
>> }
>>
>>
>> +/**
>> + * virSecurityManagerUpdateImageLabel:
>> + * @mgr: security manager object
>> + * @vm: domain definition object
>> + * @src: disk source definition to operate on
>> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
>> + *
>> + * Update security label from @src according to @flags.
>> + *
>> + * Returns: 0 on success, -1 on error.
>> + */
>> +int
>> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
>> + virDomainDef *vm,
>> + virStorageSource *src,
>> + virSecurityDomainImageLabelFlags flags)
>> +{
>> +if (mgr->drv->domainUpdateSecurityImageLabel) {
>> +int ret;
>> +virObjectLock(mgr);
>> +ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
>> +virObjectUnlock(mgr);
>> +return ret;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +
>
> Is there a reason why this needs to be inside virSecurityManager? We
> already have virSecurityMoveRememberedLabel() that lives outside of it,
> in security_util.c and conceptually this function belongs there.
>
> Michal
>
> .
>
Maybe all security managers' labels need to be updated during migration,
so I add it here.
Thanks,
Peng
Re: [PATCH 9/9] migration: update image labels in dst after migration
On 9/9/2021 7:01 PM, Michal Prívozník wrote:
> On 8/23/21 4:41 AM, Peng Liang wrote:
>> Bacause the timestamp (the uptime of the host) is used to validate the
>> remembered labels, it need to update after migration.
>>
>> Signed-off-by: Peng Liang
>> ---
>> src/qemu/qemu_migration.c | 12
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index b441d0226c8f..a5f7bd4add97 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -5624,6 +5624,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
>> qemuDomainJobInfo *jobInfo = NULL;
>> bool inPostCopy = false;
>> bool doKill = true;
>> +size_t i;
>>
>> VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
>>"cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d",
>> @@ -5831,6 +5832,17 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
>> /* Guest is successfully running, so cancel previous auto destroy */
>> qemuProcessAutoDestroyRemove(driver, vm);
>>
>> +for (i = 0; i < vm->def->ndisks; i++) {
>> +virStorageSource *src = vm->def->disks[i]->src;
>> +
>> +if (!virStorageSourceIsLocalStorage(src) || !src->path ||
>> +virFileIsSharedFS(src->path) < 0)
>
> This last check is pretty much useless. virFileIsSharedFS() returns -1
> only on failure. The way it is written completely ignores whether
> src->path is on a shared FS or not.
Oops, I'll fix it in the next version.
Thanks,
Peng
>
>> +continue;
>> +
>> +if (qemuSecurityUpdateImageLabel(driver, vm, src) < 0)
>> +VIR_WARN("Failed to update security label for %s", src->path);
>> +}
>> +
>> endjob:
>> if (!dom &&
>> !(flags & VIR_MIGRATE_OFFLINE) &&
>>
>
> Michal
>
> .
>
Re: [PATCH 7/9] migration: don't remember image labels when migrating with shared fs
On 9/9/2021 7:01 PM, Michal Prívozník wrote: > On 8/23/21 4:41 AM, Peng Liang wrote: >> When migrating with shared fs, the image labels has been remembered and >> the ownership of the image has been set in the src host. If the dst >> host remembers the ownership of the image again, the ownership of the >> image remembered in the src host (the origin ownership) will lost. >> >> Signed-off-by: Peng Liang >> --- >> src/security/security_dac.c | 32 +++- >> 1 file changed, 23 insertions(+), 9 deletions(-) >> > > > I thought that refcounting should do the trick here. At least that was > my intent when implementing this feature. I mean, the source sets > seclabels and since the domain runs just once all refcounters are equal > to 1. Then, during migration when the destination sets labels the > refcounter is (temporarily) increased to 2, but only until the source > calls restore (in which case the refcounter is decreased back to 1 again). > > Are you seeing different behaviour? When the dst try to remember the labels (in virSecuritySetRememberedLabel), it will find that the timestamp is invalid, then it will remove all labels and set a new one instead of increasing the refcounter to 2. So I add virSecurityManagerUpdateImageLabel to update labels (currently, only update timestamp) during migration. > > BTW: what FS are you using to test this? Because I'm not aware of any > shared FS that would support XATTRs. We are testing using ocfs2. Thanks, Peng > > Michal > > . >
Re: [PATCH] qemu_driver:report guest interface informations
On 8/30/21 7:35 AM, scuzhanglei wrote:
> Signed-off-by: scuzhanglei
> ---
> NEWS.rst | 5 ++
> docs/manpages/virsh.rst | 12 -
> include/libvirt/libvirt-domain.h | 1 +
> src/libvirt-domain.c | 12 +
> src/qemu/qemu_agent.c| 9 ++--
> src/qemu/qemu_agent.h| 3 +-
> src/qemu/qemu_driver.c | 88 +++-
> tests/qemuagenttest.c| 2 +-
> tools/virsh-domain.c | 6 +++
> 9 files changed, 129 insertions(+), 9 deletions(-)
The patch is more-or-less correct, however, we like to split things a
bit. In the first patch you can define the public flag and in this
specific case I'd say also do virsh change (virsh-domain.c + virsh.rst).
Or you can split that into two. Then, in the next patch you'd do the
hypervisor driver change (src/qemu/* + tests/qemuagenttest.c). And as
the very last patch you document the feature in NEWS.rst. This should
always be separate.
The reason for this split is to make it easier to backport patches
(should somebody do that).
Do you think you can do that in v2?
BTW: I'm no native speaker but I don't think "information" has a plural
form, thus s/informations/information/.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f1f961c51c..0b803c392b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18957,7 +18957,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom,
> goto endjob;
>
> agent = qemuDomainObjEnterAgent(vm);
> -ret = qemuAgentGetInterfaces(agent, ifaces);
> +ret = qemuAgentGetInterfaces(agent, ifaces, true);
> qemuDomainObjExitAgent(vm, agent);
>
> endjob:
> @@ -19903,7 +19903,8 @@ static const unsigned int
> qemuDomainGetGuestInfoSupportedTypes =
> VIR_DOMAIN_GUEST_INFO_TIMEZONE |
> VIR_DOMAIN_GUEST_INFO_HOSTNAME |
> VIR_DOMAIN_GUEST_INFO_FILESYSTEM |
> -VIR_DOMAIN_GUEST_INFO_DISKS;
> +VIR_DOMAIN_GUEST_INFO_DISKS |
> +VIR_DOMAIN_GUEST_INFO_INTERFACES;
>
> static int
> qemuDomainGetGuestInfoCheckSupport(unsigned int types,
> @@ -20102,6 +20103,69 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo,
> }
> }
>
> +static void
> +virDomainInterfaceFormatParams(virDomainInterfacePtr *ifaces,
> +int nifaces,
> +virTypedParameterPtr *params,
> +int *nparams, int * maxparams)
Indentation looks off.
> +{
> +size_t i, j;
> +const char *type = NULL;
> +
> +if (virTypedParamsAddUInt(params, nparams, maxparams,
> + "if.count", nifaces) < 0)
> + return;
> +
> +for (i = 0; i < nifaces; i++) {
> +char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.name", i);
We like to separate variable declaration block and code block with an
empty line.
> +if (virTypedParamsAddString(params, nparams, maxparams,
> +param_name, ifaces[i]->name) < 0)
> +return;
> +
> +g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.hwaddr", i);
> +if (virTypedParamsAddString(params, nparams, maxparams,
> +param_name, ifaces[i]->hwaddr) < 0)
> +return;
> +
> +g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.count", i);
> +if (virTypedParamsAddUInt(params, nparams, maxparams,
> +param_name, ifaces[i]->naddrs) < 0)
> +return;
> +
> +for (j = 0; j < ifaces[i]->naddrs; j++) {
> +switch (ifaces[i]->addrs[j].type) {
> +case VIR_IP_ADDR_TYPE_IPV4:
> +type = "ipv4";
> +break;
> +case VIR_IP_ADDR_TYPE_IPV6:
> +type = "ipv6";
> +break;
> + }
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.%zu.type", i, j);
> + if (virTypedParamsAddString(params, nparams, maxparams,
> +param_name, type) < 0)
> +return;
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.%zu.addr", i, j);
> + if (virTypedParamsAddString(params, nparams, maxparams,
> +param_name, ifaces[i]->addrs[j].addr) <
> 0)
> +return;
> +
> + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "if.%zu.addr.%zu.prefix", i, j);
> + if (virTypedParamsAddUInt(params, nparams, maxparams,
> +param_name, ifaces[i]->addrs[j].prefix)
> < 0)
> +return;
> +}
> +}
> +}
>
Michal
[libvirt PATCH 2/4] qemu: tweak error message to be more general purpose
The BIOS serial console output is currently implemented using the QEMU
'sga' device, but this is going to change in future patches, so the
error message ought to be more generically phrased.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_validate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 3789361b57..47012748e8 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1209,7 +1209,7 @@ qemuValidateDomainDef(const virDomainDef *def,
}
if (!def->nserials) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("need at least one serial port to use SGA"));
+ _("need at least one serial port to use BIOS serial
output"));
return -1;
}
}
--
2.31.1
[libvirt PATCH 4/4] qemu: stop probing for '-device sga' support
Since we no longer use '-device sga' we can stop probing for this device
in our capabilities code.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_capabilities.c | 3 +--
src/qemu/qemu_capabilities.h | 2 +-
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 -
14 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 70c3ec2f0c..f27a621f8c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -151,7 +151,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"virtio-blk-pci.ioeventfd", /* QEMU_CAPS_VIRTIO_IOEVENTFD */
/* 60 */
- "sga", /* QEMU_CAPS_SGA */
+ "sga", /* X_QEMU_CAPS_SGA */
"virtio-blk-pci.event_idx", /* QEMU_CAPS_VIRTIO_BLK_EVENT_IDX */
"virtio-net-pci.event_idx", /* QEMU_CAPS_VIRTIO_NET_EVENT_IDX */
"cache-directsync", /* X_QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC */
@@ -1219,7 +1219,6 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "virtio-scsi-device", QEMU_CAPS_VIRTIO_SCSI },
{ "megasas", QEMU_CAPS_SCSI_MEGASAS },
{ "qxl", QEMU_CAPS_DEVICE_QXL },
-{ "sga", QEMU_CAPS_SGA },
{ "scsi-block", QEMU_CAPS_SCSI_BLOCK },
{ "VGA", QEMU_CAPS_DEVICE_VGA },
{ "cirrus-vga", QEMU_CAPS_DEVICE_CIRRUS_VGA },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bc762d1916..f3379f556c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -130,7 +130,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_VIRTIO_IOEVENTFD, /* virtio-{net|blk}-pci.ioeventfd=on */
/* 60 */
-QEMU_CAPS_SGA, /* Serial Graphics Adapter */
+X_QEMU_CAPS_SGA, /* Serial Graphics Adapter */
QEMU_CAPS_VIRTIO_BLK_EVENT_IDX, /* virtio-blk-pci.event_idx */
QEMU_CAPS_VIRTIO_NET_EVENT_IDX, /* virtio-net-pci.event_idx */
X_QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC, /* Is cache=directsync supported? */
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 8a3c9c53c6..631f644144 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index 3cc5c86e4d..d74dc5ebd5 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
index ff9f88d873..2cc3c11820 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
index a80d381b71..bcc4c44d28 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
index 19b8a49394..e999d7574c 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index 841b753518..80c3e3cbed 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
index 6e3aa7f5d9..99f9375c04 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
@@ -12,7 +12,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
index 0c28645f69..03fc7d4106 1006
[libvirt PATCH 0/4] qemu: replace -device sga with -M graphics=off
SeaBIOS >= 1.11 / QEMU >= 2.11 no longer requires the 'sga' device for serial console output from the BIOS. It can be done directly with graphics=off machine option. This appears to be live migration compatible, despite changing the number of option ROMS loaded, though I need a little more testing to fully confirm this. Daniel P. Berrangé (4): qemu: prevent use of on non-x86 arches qemu: tweak error message to be more general purpose qemu: switch to use -M graphics=off instead of -device sga qemu: stop probing for '-device sga' support src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 25 --- src/qemu/qemu_validate.c | 15 --- .../caps_2.11.0.x86_64.xml| 1 - .../caps_2.12.0.x86_64.xml| 1 - .../caps_3.0.0.x86_64.xml | 1 - .../caps_3.1.0.x86_64.xml | 1 - .../caps_4.0.0.x86_64.xml | 1 - .../caps_4.1.0.x86_64.xml | 1 - .../caps_4.2.0.x86_64.xml | 1 - .../caps_5.0.0.x86_64.xml | 1 - .../caps_5.1.0.x86_64.xml | 1 - .../caps_5.2.0.x86_64.xml | 1 - .../caps_6.0.0.x86_64.xml | 1 - .../caps_6.1.0.x86_64.xml | 1 - tests/qemuxml2argvdata/bios.args | 3 +-- tests/qemuxml2argvtest.c | 3 +-- 18 files changed, 25 insertions(+), 38 deletions(-) -- 2.31.1
[libvirt PATCH 1/4] qemu: prevent use of on non-x86 arches
The config results in use of the '-device sga'
QEMU options. This in turn causes QEMU go load the sgabios.bin option
ROM, which contains x86 machine code. This cannot work on non-x86
arches, thus we should block the bad config.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_validate.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index d8f39b6bdd..3789361b57 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1194,6 +1194,14 @@ qemuValidateDomainDef(const virDomainDef *def,
/* Serial graphics adapter */
if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) {
+/* -device sga is only sane on x86, since the option ROM it
+ * loads contains x86 machine code.
+ */
+if (!ARCH_IS_X86(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("BIOS serial console only supported on x86
architectures"));
+return -1;
+}
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("qemu does not support SGA"));
--
2.31.1
[libvirt PATCH 3/4] qemu: switch to use -M graphics=off instead of -device sga
SeaBIOS >= 1.11 has built-in support for outputting to the serial
console when QEMU sets -M graphics=off. Our minimum QEMU version
is 2.11.0, which bundled SeaBIOS 1.11. Thus we have no need to
use '-device sga' anymore.
This change results in a slight layout difference for option ROMs
in memory, however, it does not affect the migration data stream
format on the wire and once migration is complete the target QEMU
memory layout for ROMs matches the source QEMU once again.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c | 25 ++---
src/qemu/qemu_validate.c | 13 ++---
tests/qemuxml2argvdata/bios.args | 3 +--
tests/qemuxml2argvtest.c | 3 +--
4 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b230314f7f..4d29313f45 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5956,18 +5956,6 @@ qemuBuildVMGenIDCommandLine(virCommand *cmd,
}
-static int
-qemuBuildSgaCommandLine(virCommand *cmd,
-const virDomainDef *def)
-{
-/* Serial graphics adapter */
-if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES)
-virCommandAddArgList(cmd, "-device", "sga", NULL);
-
-return 0;
-}
-
-
static char *
qemuBuildClockArgStr(virDomainClockDef *def)
{
@@ -7054,6 +7042,16 @@ qemuBuildMachineCommandLine(virCommand *cmd,
virBufferAsprintf(&buf, ",memory-backend=%s", defaultRAMid);
}
+/* On x86 targets, graphics=off activates the serial console
+ * output mode in the firmware. On non-x86 targets it has
+ * various other undesirable effects that we certainly do
+ * not want to have. We rely on the validation code to have
+ * blocked useserial=yes on non-x86
+ */
+if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) {
+virBufferAddLit(&buf, ",graphics=off");
+}
+
virCommandAddArgBuffer(cmd, &buf);
return 0;
@@ -10553,9 +10551,6 @@ qemuBuildCommandLine(virQEMUDriver *driver,
virCommandAddArg(cmd, "-no-user-config");
virCommandAddArg(cmd, "-nodefaults");
-if (qemuBuildSgaCommandLine(cmd, def) < 0)
-return NULL;
-
if (qemuBuildMonitorCommandLine(logManager, secManager, cmd, cfg, def,
priv) < 0)
return NULL;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 47012748e8..9d93f373ab 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1194,19 +1194,18 @@ qemuValidateDomainDef(const virDomainDef *def,
/* Serial graphics adapter */
if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) {
-/* -device sga is only sane on x86, since the option ROM it
- * loads contains x86 machine code.
+/* On x86 -machine graphics=off toggles the use of the
+ * serial console in SeaBIOS (and theoretically other
+ * firmwares).
+ * On non-x86, it has also sorts of other effects
+ * on QEMU device models created and so we don't
+ * want to allow its use.
*/
if (!ARCH_IS_X86(def->os.arch)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("BIOS serial console only supported on x86
architectures"));
return -1;
}
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("qemu does not support SGA"));
-return -1;
-}
if (!def->nserials) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("need at least one serial port to use BIOS serial
output"));
diff --git a/tests/qemuxml2argvdata/bios.args b/tests/qemuxml2argvdata/bios.args
index 7356d1626d..7d831a716d 100644
--- a/tests/qemuxml2argvdata/bios.args
+++ b/tests/qemuxml2argvdata/bios.args
@@ -10,7 +10,7 @@ QEMU_AUDIO_DRV=none \
/usr/bin/qemu-system-i386 \
-name guest=test-bios,debug-threads=on \
-S \
--machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off,graphics=off \
-bios /usr/share/seabios/bios.bin \
-m 1024 \
-realtime mlock=off \
@@ -19,7 +19,6 @@ QEMU_AUDIO_DRV=none \
-display none \
-no-user-config \
-nodefaults \
--device sga \
-chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-test-bios/monitor.sock,server=on,wait=off
\
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c67214d01e..4d82267598 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1192,8 +1192,7 @@ mymain(void)
DO_TEST_PARSE_ERROR_NOCAPS("reboot-timeout-enabled");
DO_TEST("bios",
-QEMU_CAPS_DEVICE_ISA_SERIAL,
-QEMU_CAPS_SGA);
+QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_NOCAPS("bios-nvram");
DO_TEST("bios-nvram-secure",
QEMU_CAPS_DEVICE_DMI_TO_PCI_BR
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
On 8/23/21 4:41 AM, Peng Liang wrote:
> Signed-off-by: Peng Liang
> ---
> src/libvirt_private.syms| 1 +
> src/security/security_driver.h | 5 +
> src/security/security_manager.c | 29 +
> src/security/security_manager.h | 5 +
> 4 files changed, 40 insertions(+)
>
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 9906c1691d0f..b580704d3abf 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager
> *mgr,
> }
>
>
> +/**
> + * virSecurityManagerUpdateImageLabel:
> + * @mgr: security manager object
> + * @vm: domain definition object
> + * @src: disk source definition to operate on
> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
> + *
> + * Update security label from @src according to @flags.
> + *
> + * Returns: 0 on success, -1 on error.
> + */
> +int
> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
> + virDomainDef *vm,
> + virStorageSource *src,
> + virSecurityDomainImageLabelFlags flags)
> +{
> +if (mgr->drv->domainUpdateSecurityImageLabel) {
> +int ret;
> +virObjectLock(mgr);
> +ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
> +virObjectUnlock(mgr);
> +return ret;
> +}
> +
> +return 0;
> +}
> +
> +
Is there a reason why this needs to be inside virSecurityManager? We
already have virSecurityMoveRememberedLabel() that lives outside of it,
in security_util.c and conceptually this function belongs there.
Michal
Re: [PATCH 9/9] migration: update image labels in dst after migration
On 8/23/21 4:41 AM, Peng Liang wrote:
> Bacause the timestamp (the uptime of the host) is used to validate the
> remembered labels, it need to update after migration.
>
> Signed-off-by: Peng Liang
> ---
> src/qemu/qemu_migration.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index b441d0226c8f..a5f7bd4add97 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5624,6 +5624,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
> qemuDomainJobInfo *jobInfo = NULL;
> bool inPostCopy = false;
> bool doKill = true;
> +size_t i;
>
> VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
>"cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d",
> @@ -5831,6 +5832,17 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
> /* Guest is successfully running, so cancel previous auto destroy */
> qemuProcessAutoDestroyRemove(driver, vm);
>
> +for (i = 0; i < vm->def->ndisks; i++) {
> +virStorageSource *src = vm->def->disks[i]->src;
> +
> +if (!virStorageSourceIsLocalStorage(src) || !src->path ||
> +virFileIsSharedFS(src->path) < 0)
This last check is pretty much useless. virFileIsSharedFS() returns -1
only on failure. The way it is written completely ignores whether
src->path is on a shared FS or not.
> +continue;
> +
> +if (qemuSecurityUpdateImageLabel(driver, vm, src) < 0)
> +VIR_WARN("Failed to update security label for %s", src->path);
> +}
> +
> endjob:
> if (!dom &&
> !(flags & VIR_MIGRATE_OFFLINE) &&
>
Michal
Re: [PATCH 7/9] migration: don't remember image labels when migrating with shared fs
On 8/23/21 4:41 AM, Peng Liang wrote: > When migrating with shared fs, the image labels has been remembered and > the ownership of the image has been set in the src host. If the dst > host remembers the ownership of the image again, the ownership of the > image remembered in the src host (the origin ownership) will lost. > > Signed-off-by: Peng Liang > --- > src/security/security_dac.c | 32 +++- > 1 file changed, 23 insertions(+), 9 deletions(-) > I thought that refcounting should do the trick here. At least that was my intent when implementing this feature. I mean, the source sets seclabels and since the domain runs just once all refcounters are equal to 1. Then, during migration when the destination sets labels the refcounter is (temporarily) increased to 2, but only until the source calls restore (in which case the refcounter is decreased back to 1 again). Are you seeing different behaviour? BTW: what FS are you using to test this? Because I'm not aware of any shared FS that would support XATTRs. Michal
Re: [PATCHv4 0/9] ch: Add Console support
On Wed, Sep 08, 2021 at 11:01:14AM -0700, William Douglas wrote: > This series enables console support in the cloud-hypervisor driver. > > Cloud-hypervisor only supports a single console or serial device at a > time, hence the checks to ensure the domain configuration is only > passing one or the other. The comments I've had are fairly simple, and I made them locally when testing and reviewing the code. So I'll just push your series directly with the suggested changes, to avoid need to repost a v5 Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 3/9] ch_monitor: Update virCHMonitorGet to handle accept a response
On Thu, Sep 09, 2021 at 09:48:48AM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 08, 2021 at 11:01:17AM -0700, William Douglas wrote:
> > The virCHMonitorGet function needed to be able to return data from the
> > hypervisor. This functionality is needed in order for the driver to
> > support PTY enablement and getting details about the VM state.
> >
> > Signed-off-by: William Douglas
> > ---
> > src/ch/ch_monitor.c | 46 +++--
> > 1 file changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> > index b4bc10bfcf..44b99ef07a 100644
> > --- a/src/ch/ch_monitor.c
> > +++ b/src/ch/ch_monitor.c
> > @@ -611,12 +611,36 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const
> > char *endpoint)
> > return ret;
> > }
> >
> > +struct curl_data {
> > +char *content;
> > +size_t size;
> > +};
> > +
> > +static size_t
> > +curl_callback(void *contents, size_t size, size_t nmemb, void *userp)
> > +{
> > +size_t content_size = size * nmemb;
> > +struct curl_data *data = (struct curl_data *)userp;
>
> FWIW, the type cast here is redundant as 'void *' can be directly
> assigned to/from any other type.
>
> > +
> > +if (content_size == 0)
> > +return content_size;
> > +
> > +data->content = g_realloc(data->content, data->size + content_size);
> > +
> > +memcpy(&(data->content[data->size]), contents, content_size);
> > +data->size += content_size;
> > +
> > +return content_size;
> > +}
> > +
> > static int
> > -virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
> > +virCHMonitorGet(virCHMonitor *mon, const char *endpoint, virJSONValue
> > **response)
> > {
> > g_autofree char *url = NULL;
> > int responseCode = 0;
> > int ret = -1;
> > +struct curl_slist *headers = NULL;
> > +struct curl_data data = {0};
> >
> > url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
> >
> > @@ -628,12 +652,30 @@ virCHMonitorGet(virCHMonitor *mon, const char
> > *endpoint)
> > curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH,
> > mon->socketpath);
> > curl_easy_setopt(mon->handle, CURLOPT_URL, url);
> >
> > +if (response) {
> > +headers = curl_slist_append(headers, "Accept: application/json");
> > +headers = curl_slist_append(headers, "Content-Type:
> > application/json");
> > +curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
> > +curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION,
> > curl_callback);
> > +curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data);
> > +}
> > +
> > responseCode = virCHMonitorCurlPerform(mon->handle);
> >
> > virObjectUnlock(mon);
> >
> > -if (responseCode == 200 || responseCode == 204)
> > +if (responseCode == 200 || responseCode == 204) {
> > ret = 0;
> > +if (response) {
> > +data.content = g_realloc(data.content, data.size + 1);
> > +data.content[data.size] = 0;
> > +*response = virJSONValueFromString(data.content);
>
> Oh, well need to add:
>
> if (!*response)
>return -1;
>
> in case JSON parsing fails
Actually slight more to avoid a leak.
@@ -665,14 +665,17 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint,
virJSONValue **response
virObjectUnlock(mon);
if (responseCode == 200 || responseCode == 204) {
-ret = 0;
if (response) {
data.content = g_realloc(data.content, data.size + 1);
data.content[data.size] = 0;
*response = virJSONValueFromString(data.content);
+if (!*response)
+goto cleanup;
}
+ret = 0;
}
+ cleanup:
g_free(data.content);
/* reset the libcurl handle to avoid leaking a stack pointer to data */
curl_easy_reset(mon->handle);
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 00/24] virstoragetest: Re-instate testing of images without backing format
On a Thursday in 2021, Peter Krempa wrote: Most of the series are refactors to make virstoragetest less archaic, the last commit then re-introduces testing of images which don't have backing format recorded in the metadata which can't be formatted using qemu-img any more but have security implications if we'd mishandle them. Peter Krempa (24): virstoragetest: Drop testing of RBD backends via parsing real images virstoragetest: Drop testing of NBD backends via parsing real images [...] virstoragetest: Remove pointless goto from mymain virstoragetest: Reinstate testing of images without 'backing_fmt' build-aux/syntax-check.mk | 2 +- tests/testutils.c | 30 + tests/testutils.h | 3 + [...] tests/virstoragetestdata/out/raw-auto | 9 + tests/virstoragetestdata/out/raw-raw | 9 + 42 files changed, 580 insertions(+), 628 deletions(-) create mode 100644 tests/virstoragetestdata/images/loop-1.qcow2 [...] Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 16/24] virstoragetest: Stop rewriting images in 'mymain'
On a Thursday in 2021, Peter Krempa wrote: For testing of real images formatted by 'qemu-img' it's now sufficient to format them once without the need to rewrtie them since we use the real images only for testing of one scenario. This allows us to also remove moust of the global variables holding the *most Jano path to the images which was necessary when they were being rewritten. Signed-off-by: Peter Krempa --- tests/virstoragetest.c | 33 - 1 file changed, 4 insertions(+), 29 deletions(-) signature.asc Description: PGP signature
Re: [PATCH 1/9] ch_domain: Add virChrdevs for console support
On Wed, Sep 08, 2021 at 11:01:15AM -0700, William Douglas wrote:
> Add and initialize a virChrdevs to the _virCHDomainObjPrivate
> structure in order to eventually track the consoles in use by a domain.
>
> Signed-off-by: William Douglas
> ---
> src/ch/ch_domain.c | 7 +++
> src/ch/ch_domain.h | 3 +++
> 2 files changed, 10 insertions(+)
>
> diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
> index 780a46ba00..a6b87e28e5 100644
> --- a/src/ch/ch_domain.c
> +++ b/src/ch/ch_domain.c
> @@ -22,6 +22,7 @@
>
> #include "ch_domain.h"
> #include "viralloc.h"
> +#include "virchrdev.h"
> #include "virlog.h"
> #include "virtime.h"
>
> @@ -146,6 +147,11 @@ virCHDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED)
> return NULL;
> }
>
> +if (!(priv->chrdevs = virChrdevAlloc())) {
Needs
virCHDomainObjFreeJob(priv);
> +g_free(priv);
> +return NULL;
> +}
> +
> return priv;
> }
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 2/9] ch_monitor: Make unused function static
On Wed, Sep 08, 2021 at 11:01:16AM -0700, William Douglas wrote:
> The virCHMonitorGet function wasn't in use and was declared as
> non-static (though not in a header file). This function isn't going to
> be used outside of the monitor though so remove the initial
> declaration and define the function to be static.
>
> Future work should adjust this function to allow reading VM state
> needed for PTY enablement.
>
> Signed-off-by: William Douglas
> ---
> src/ch/ch_monitor.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index 0f8752d1ed..b4bc10bfcf 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -54,7 +54,6 @@ VIR_ONCE_GLOBAL_INIT(virCHMonitor);
>
> int virCHMonitorShutdownVMM(virCHMonitor *mon);
> int virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint);
> -int virCHMonitorGet(virCHMonitor *mon, const char *endpoint);
>
> static int
> virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
> @@ -612,7 +611,7 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char
> *endpoint)
> return ret;
> }
>
> -int
> +static int
> virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
> {
> g_autofree char *url = NULL;
This patch breaks the compile because we end up with an unused static
function. It just needs to be moved a little later in the patch series.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH 11/24] virstoragetest: Use preformatted file for testing missing backing store
Similarly to previous ones, this one doesn't need to be created by
qemu-img in order for the test to make sense.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 12 +++-
.../images/qcow2_qcow2-missing.qcow2 | Bin 0 -> 196616 bytes
2 files changed, 3 insertions(+), 9 deletions(-)
create mode 100644 tests/virstoragetestdata/images/qcow2_qcow2-missing.qcow2
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 8d3dde265f..34aff3e6dd 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -525,16 +525,10 @@ mymain(void)
/* qcow2 with a longer backing chain */
TEST_CHAIN("qcow2-qcow2_qcow2-qcow2_raw-raw", abswrap,
VIR_STORAGE_FILE_QCOW2, EXP_PASS);
-/* Rewrite qcow2 to a missing backing file, with backing type */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "qcow2", "-b", datadir "/bogus",
- "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
/* Qcow2 file with missing backing file but specified type */
-TEST_CHAIN("qcow2-qcow2_missing", absqcow2, VIR_STORAGE_FILE_QCOW2,
EXP_FAIL);
+TEST_CHAIN("qcow2-qcow2_missing",
+ abs_srcdir
"/virstoragetestdata/images/qcow2_qcow2-missing.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_FAIL);
/* Qcow2 file with backing protocol instead of file */
TEST_CHAIN("qcow2-qcow2_nbd-raw",
diff --git a/tests/virstoragetestdata/images/qcow2_qcow2-missing.qcow2
b/tests/virstoragetestdata/images/qcow2_qcow2-missing.qcow2
new file mode 100644
index
..cb9afb25dcd0b489fe848160255d9e5b427ddf0f
GIT binary patch
literal 196616
zcmeIuOHPA87y#gb>fR%;cIhEZOx(C~O(`l#1*9~!t~{T|@DwI)oW9by5SuQW4>HWa
z{14{4yT17jA&laPS9%d2=W(p&c%5yg5R&y`8?}AgeA`9*J+F$iyY0{AzCQM>UW{Tb5b;2H*fnxGt%<4FS|OQW|O?nvxjP04mKDS&2!t8
zvsu;DS-)(Dqa)R-emyBC+3i(U)_qr%vw;RlnC5TGtScXiW?uJ0q3bGXh%(Q4`)}a3
zXPR{RBKvjbfxp>rjhn~$p*{9lRs;wTAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PGjrKzcGKaS#Fo
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5I9yK37h-ae0c;25FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
IAaGIwKRetu*Z=?k
literal 0
HcmV?d1
--
2.31.1
[PATCH 20/24] virstoragetest: testPrepImages: Use 'qemu-img' to format 'raw' image
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 90dde512cf..c258bc1709 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -86,9 +86,9 @@ static int
testPrepImages(void)
{
int ret = EXIT_FAILURE;
+g_autoptr(virCommand) cmdraw = NULL;
g_autoptr(virCommand) cmdqcow2 = NULL;
g_autoptr(virCommand) cmdwrap = NULL;
-g_autofree char *buf = NULL;
g_autofree char *absraw = g_strdup_printf("%s/raw", datadir);
g_autofree char *absqcow2 = g_strdup_printf("%s/qcow2", datadir);
g_autofree char *qemuimg = virFindFileInPath("qemu-img");
@@ -111,11 +111,11 @@ testPrepImages(void)
goto cleanup;
}
-buf = g_strdup_printf("%1024d", 0);
-if (virFileWriteStr("raw", buf, 0600) < 0) {
-fprintf(stderr, "unable to create raw file\n");
-goto cleanup;
-}
+cmdraw = virCommandNewArgList(qemuimg, "create",
+ "-f", "raw",
+ absraw, "1k", NULL);
+if (virCommandRun(cmdraw, NULL) < 0)
+goto skip;
/* Create a qcow2 wrapping relative raw; later on, we modify its
* metadata to test other configurations */
--
2.31.1
[PATCH 24/24] virstoragetest: Reinstate testing of images without 'backing_fmt'
There are important security implications when we'd misprobe those
images. This commit reinstates the tests removed by commit 979d1ba3ae13
since 'qemu-img' refused to format them.
With the new testing approach with stored images we won't run into that
problem.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 14 +
tests/virstoragetestdata/images/qcow2 | Bin 0 -> 196616 bytes
.../images/qcow2_qcow2-auto.qcow2 | Bin 0 -> 196616 bytes
.../images/qcow2_qcow2-auto_qcow2-auto.qcow2 | Bin 0 -> 196616 bytes
.../images/qcow2_qcow2-qcow2_qcow2-auto.qcow2 | Bin 0 -> 196616 bytes
.../images/qcow2_qcow2-qcow2_raw-auto.qcow2 | Bin 0 -> 196616 bytes
.../images/qcow2_raw-auto.qcow2 | Bin 0 -> 196616 bytes
.../out/qcow2-qcow2_qcow2-auto| 19
.../out/qcow2-qcow2_qcow2-qcow2_qcow2-auto| 29 ++
.../out/qcow2-qcow2_qcow2-qcow2_raw-auto | 29 ++
10 files changed, 91 insertions(+)
create mode 100644 tests/virstoragetestdata/images/qcow2
create mode 100644 tests/virstoragetestdata/images/qcow2_qcow2-auto.qcow2
create mode 100644
tests/virstoragetestdata/images/qcow2_qcow2-auto_qcow2-auto.qcow2
create mode 100644
tests/virstoragetestdata/images/qcow2_qcow2-qcow2_qcow2-auto.qcow2
create mode 100644
tests/virstoragetestdata/images/qcow2_qcow2-qcow2_raw-auto.qcow2
create mode 100644 tests/virstoragetestdata/images/qcow2_raw-auto.qcow2
create mode 100644 tests/virstoragetestdata/out/qcow2-qcow2_qcow2-auto
create mode 100644
tests/virstoragetestdata/out/qcow2-qcow2_qcow2-qcow2_qcow2-auto
create mode 100644
tests/virstoragetestdata/out/qcow2-qcow2_qcow2-qcow2_raw-auto
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 513ffdeb41..ec185d8660 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -464,6 +464,20 @@ mymain(void)
testCleanupImages();
+/* Test various combinations of qcow2 images with missing 'backing_format'
*/
+TEST_CHAIN("qcow2-qcow2_qcow2-qcow2_qcow2-auto",
+ abs_srcdir
"/virstoragetestdata/images/qcow2_qcow2-qcow2_qcow2-auto.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_PASS);
+TEST_CHAIN("qcow2-qcow2_qcow2-qcow2_raw-auto",
+ abs_srcdir
"/virstoragetestdata/images/qcow2_qcow2-qcow2_raw-auto.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_PASS);
+TEST_CHAIN("qcow2-qcow2_qcow2-auto_qcow2-auto",
+ abs_srcdir
"/virstoragetestdata/images/qcow2_qcow2-auto_qcow2-auto.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_FAIL);
+TEST_CHAIN("qcow2-qcow2_qcow2-auto",
+ abs_srcdir "/virstoragetestdata/images/qcow2_qcow2-auto.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_PASS);
+
/* Qcow2 file with missing backing file but specified type */
TEST_CHAIN("qcow2-qcow2_missing",
abs_srcdir
"/virstoragetestdata/images/qcow2_qcow2-missing.qcow2",
diff --git a/tests/virstoragetestdata/images/qcow2
b/tests/virstoragetestdata/images/qcow2
new file mode 100644
index
..31a144e4b947692c834363797fa34457edc2094c
GIT binary patch
literal 196616
zcmeIuK~94}6adhH_5dDX&fpJXO4{o6X#@$NdNHV;kO
z%`_;&rh41QA+1;4bDQT%PrH=iAhWIdKMTKmmc>xL#9wEg`5P$$1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72%Hfp&%}iZ5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAVA=91d1@;mvdq#B|v}x
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
a009C72oNAZfB*pk1PBlyK!5;&8G$bzfG}YI
literal 0
HcmV?d1
diff --git a/tests/virstoragetestdata/images/qcow2_qcow2-auto.qcow2
b/tests/virstoragetestdata/images/qcow2_qcow2-auto.qcow2
new file mode 100644
index
..490482150d476032cfccd91be403f15430682f67
GIT binary patch
literal 196616
zcmeIuK~94}6adhH>fR%8h91Ji#EmQ02sDzkfOMK#S03BMjnL9GE=2b}$S{AH&;QJO
zxV`%fAuNV5T&cWgm875LP(Cmlr^nR@6H)N&tsA9y#2iRcpS>ThfvmiJY-LG
zJa5gCAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1P
[PATCH 23/24] virstoragetest: Remove pointless goto from mymain
Improve the error message and abort the test. Continuing here is not
desired as without chdiring into the appropriate directory the test
would fail anyways and worse could attempt stat-ing random files on the
host.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index bbeb5ecd88..513ffdeb41 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -502,8 +502,9 @@ mymain(void)
/* setup data for backing chain lookup testing */
if (chdir(abs_srcdir "/virstoragetestdata/lookup") < 0) {
-fprintf(stderr, "unable to test relative backing chains\n");
-goto cleanup;
+VIR_TEST_VERBOSE("failed to chdir into '%s'\n",
+ abs_srcdir "/virstoragetestdata/lookup");
+return EXIT_FAILURE;
}
memset(fakeChain, 0, sizeof(fakeChain));
@@ -1171,7 +1172,6 @@ mymain(void)
#endif /* WITH_YAJL */
- cleanup:
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.31.1
[PATCH 22/24] virstoragetest: Don't skip the whole test when qemu-img fails to format images
We have plenty of other work to do in this test. Skip only the real
image testing case when we can't find qemu-img or it failed to format
the image.
This allows us to also remove the last global variable in the test and
move the creation and cleanup of the images closer to the actual test.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 73 --
1 file changed, 27 insertions(+), 46 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index d9ab630600..bbeb5ecd88 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -37,13 +37,9 @@ VIR_LOG_INIT("tests.storagetest");
#define datadir abs_builddir "/virstoragedata"
-static char *abswrap;
-
static void
testCleanupImages(void)
{
-VIR_FREE(abswrap);
-
if (chdir(abs_builddir) < 0) {
fprintf(stderr, "unable to return to correct directory, refusing to "
"clean up %s\n", datadir);
@@ -82,75 +78,60 @@ testStorageFileGetMetadata(const char *path,
return g_steal_pointer(&def);
}
-static int
+static char *
testPrepImages(void)
{
-int ret = EXIT_FAILURE;
g_autoptr(virCommand) cmdraw = NULL;
g_autoptr(virCommand) cmdqcow2 = NULL;
g_autoptr(virCommand) cmdwrap = NULL;
g_autofree char *absraw = g_strdup_printf("%s/raw", datadir);
g_autofree char *absqcow2 = g_strdup_printf("%s/qcow2", datadir);
+g_autofree char *abswrap = g_strdup_printf("%s/wrap", datadir);
g_autofree char *qemuimg = virFindFileInPath("qemu-img");
if (!qemuimg)
-goto skip;
+return NULL;
/* Clean up from any earlier failed tests */
virFileDeleteTree(datadir);
-abswrap = g_strdup_printf("%s/wrap", datadir);
-
if (g_mkdir_with_parents(datadir, 0777) < 0) {
-fprintf(stderr, "unable to create directory %s\n", datadir);
-goto cleanup;
-}
-
-if (chdir(datadir) < 0) {
-fprintf(stderr, "unable to test relative backing chains\n");
-goto cleanup;
+VIR_TEST_VERBOSE("unable to create directory '%s'\n", datadir);
+return NULL;
}
+/* create the folowing real backing chain with qcow2 images with absolute
+ * backing and different qcow2 versions:
+ * datadir/raw <- datadir/qcow2 (qcow2v2) <- datadir/wrap (qcow2v3) */
cmdraw = virCommandNewArgList(qemuimg, "create",
"-f", "raw",
absraw, "1k", NULL);
-if (virCommandRun(cmdraw, NULL) < 0)
-goto skip;
-/* Create a qcow2 wrapping relative raw; later on, we modify its
- * metadata to test other configurations */
cmdqcow2 = virCommandNewArgList(qemuimg, "create",
"-f", "qcow2",
"-F", "raw",
"-b", absraw,
"-o", "compat=0.10",
absqcow2, NULL);
-if (virCommandRun(cmdqcow2, NULL) < 0)
-goto skip;
-/* Create a second qcow2 wrapping the first, to be sure that we
- * can correctly avoid insecure probing. */
cmdwrap = virCommandNewArgList(qemuimg, "create",
"-f", "qcow2",
"-F", "qcow2",
"-b", absqcow2,
"-o", "compat=1.1",
abswrap, NULL);
-if (virCommandRun(cmdwrap, NULL) < 0)
-goto skip;
-ret = 0;
- cleanup:
-if (ret)
-testCleanupImages();
-return ret;
+if (virCommandRun(cmdraw, NULL) < 0 ||
+virCommandRun(cmdqcow2, NULL) < 0 ||
+virCommandRun(cmdwrap, NULL) < 0) {
+VIR_TEST_VERBOSE("failed to create backing chain in '%s'\n", datadir);
+return NULL;
+}
- skip:
-fputs("qemu-img is too old; skipping this test\n", stderr);
-ret = EXIT_AM_SKIP;
-goto cleanup;
+return g_steal_pointer(&abswrap);
}
+
enum {
EXP_PASS = 0,
EXP_FAIL = 1,
@@ -432,11 +413,12 @@ testBackingParse(const void *args)
static int
mymain(void)
{
-int ret;
+int ret = 0;
struct testChainData data;
struct testLookupData data2;
struct testPathRelativeBacking data4;
struct testBackingParseData data5;
+g_autofree char *realchain = NULL;
virStorageSource fakeChain[4];
virStorageSource *chain = &fakeChain[0];
virStorageSource *chain2 = &fakeChain[1];
@@ -445,11 +427,6 @@ mymain(void)
if (storageRegisterAll() < 0)
return EXIT_FAILURE;
-/* Prep some files with qemu-img; if that is not found on PATH, or
- * if it lacks support for qcow2 and qed, skip this test. */
-if ((ret = testPrepImages()) != 0)
-return ret;
-
#define TEST_CHAIN(testname, start, format, flags) \
do { \
data = (struct testChainData){ testname, start, format, flags }; \
@@ -477,8 +454,15 @@ mymain
[PATCH 19/24] virstoragetest: testPrepImages: Don't reuse 'cmd' pointer
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 955ac64e0b..90dde512cf 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -86,7 +86,8 @@ static int
testPrepImages(void)
{
int ret = EXIT_FAILURE;
-g_autoptr(virCommand) cmd = NULL;
+g_autoptr(virCommand) cmdqcow2 = NULL;
+g_autoptr(virCommand) cmdwrap = NULL;
g_autofree char *buf = NULL;
g_autofree char *absraw = g_strdup_printf("%s/raw", datadir);
g_autofree char *absqcow2 = g_strdup_printf("%s/qcow2", datadir);
@@ -118,25 +119,24 @@ testPrepImages(void)
/* Create a qcow2 wrapping relative raw; later on, we modify its
* metadata to test other configurations */
-cmd = virCommandNewArgList(qemuimg, "create",
- "-f", "qcow2",
- "-F", "raw",
- "-b", absraw,
- "-o", "compat=0.10",
- absqcow2, NULL);
-if (virCommandRun(cmd, NULL) < 0)
+cmdqcow2 = virCommandNewArgList(qemuimg, "create",
+"-f", "qcow2",
+"-F", "raw",
+"-b", absraw,
+"-o", "compat=0.10",
+absqcow2, NULL);
+if (virCommandRun(cmdqcow2, NULL) < 0)
goto skip;
/* Create a second qcow2 wrapping the first, to be sure that we
* can correctly avoid insecure probing. */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "create",
- "-f", "qcow2",
- "-F", "qcow2",
- "-b", absqcow2,
- "-o", "compat=1.1",
- abswrap, NULL);
-if (virCommandRun(cmd, NULL) < 0)
+cmdwrap = virCommandNewArgList(qemuimg, "create",
+ "-f", "qcow2",
+ "-F", "qcow2",
+ "-b", absqcow2,
+ "-o", "compat=1.1",
+ abswrap, NULL);
+if (virCommandRun(cmdwrap, NULL) < 0)
goto skip;
ret = 0;
--
2.31.1
[PATCH 21/24] virstoragetest: testStorageChain: Skip test if filename is NULL
Prepare the test runner for skipping individual tests if images can't be
formatted rather than the whole virstoragetest.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index c258bc1709..d9ab630600 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -176,6 +176,11 @@ testStorageChain(const void *args)
g_autofree char *expectpath =
g_strdup_printf("%s/virstoragetestdata/out/%s",
abs_srcdir, data->testname);
+/* If the filename is NULL it means that the images couldn't be created,
+ * thus skip this particular test. */
+if (!data->start)
+return EXIT_AM_SKIP;
+
meta = testStorageFileGetMetadata(data->start, data->format, -1, -1);
if (!meta) {
if (data->flags & EXP_FAIL) {
--
2.31.1
[PATCH 18/24] virstoragetest: Assume that 'qemu-img' supports '-o compat='
All supported qemu versions have the parameter, so we don't need to
check. This allows us to simplify the code used for formating real
images for virstoragetest.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 31 +--
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 9a7905e28d..955ac64e0b 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -86,7 +86,6 @@ static int
testPrepImages(void)
{
int ret = EXIT_FAILURE;
-bool compat = false;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *buf = NULL;
g_autofree char *absraw = g_strdup_printf("%s/raw", datadir);
@@ -99,18 +98,6 @@ testPrepImages(void)
/* Clean up from any earlier failed tests */
virFileDeleteTree(datadir);
-/* See if qemu-img supports '-o compat=xxx'. If so, we force the
- * use of both v2 and v3 files; if not, it is v2 only but the test
- * still works. */
-cmd = virCommandNewArgList(qemuimg, "create", "-f", "qcow2",
- "-o?", "/dev/null", NULL);
-virCommandSetOutputBuffer(cmd, &buf);
-if (virCommandRun(cmd, NULL) < 0)
-goto skip;
-if (strstr(buf, "compat "))
-compat = true;
-VIR_FREE(buf);
-
abswrap = g_strdup_printf("%s/wrap", datadir);
if (g_mkdir_with_parents(datadir, 0777) < 0) {
@@ -131,24 +118,24 @@ testPrepImages(void)
/* Create a qcow2 wrapping relative raw; later on, we modify its
* metadata to test other configurations */
-virCommandFree(cmd);
cmd = virCommandNewArgList(qemuimg, "create",
"-f", "qcow2",
"-F", "raw",
- "-b", absraw, NULL);
-if (compat)
-virCommandAddArgList(cmd, "-o", "compat=0.10", NULL);
-virCommandAddArg(cmd, "qcow2");
+ "-b", absraw,
+ "-o", "compat=0.10",
+ absqcow2, NULL);
if (virCommandRun(cmd, NULL) < 0)
goto skip;
/* Create a second qcow2 wrapping the first, to be sure that we
* can correctly avoid insecure probing. */
virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "create", "-f", "qcow2", NULL);
-virCommandAddArgFormat(cmd, "-obacking_file=%s,backing_fmt=qcow2%s",
- absqcow2, compat ? ",compat=1.1" : "");
-virCommandAddArg(cmd, "wrap");
+cmd = virCommandNewArgList(qemuimg, "create",
+ "-f", "qcow2",
+ "-F", "qcow2",
+ "-b", absqcow2,
+ "-o", "compat=1.1",
+ abswrap, NULL);
if (virCommandRun(cmd, NULL) < 0)
goto skip;
--
2.31.1
[PATCH 15/24] virstoragetest: Unify testing of QCOW2 images with absolute backing
We have 3 test cases for this currently:
1) "qcow2->raw"
1.1) VIR_STORAGE_FILE_QCOW2 as top level format
1.2) VIR_STORAGE_FILE_AUTO as top level format
2) "wrap->qcow2->raw" whith just VIR_STORAGE_FILE_QCOW2
This patch adds also testing of VIR_STORAGE_FILE_AUTO for case 2) and
removes both 1) subcases as they are being actually tested as part of
2).
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 7 ++-
...raw-raw => qcow2-auto_qcow2-qcow2_raw-raw} | 2 +-
.../out/qcow2-qcow2_raw-raw | 19 ---
3 files changed, 3 insertions(+), 25 deletions(-)
rename tests/virstoragetestdata/out/{qcow2-auto_raw-raw =>
qcow2-auto_qcow2-qcow2_raw-raw} (74%)
delete mode 100644 tests/virstoragetestdata/out/qcow2-qcow2_raw-raw
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 9af8795492..475ff23ce0 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -512,12 +512,9 @@ mymain(void)
if (virCommandRun(cmd, NULL) < 0)
ret = -1;
-/* Qcow2 file with raw as absolute backing, backing format provided */
-TEST_CHAIN("qcow2-qcow2_raw-raw", absqcow2, VIR_STORAGE_FILE_QCOW2,
EXP_PASS);
-TEST_CHAIN("qcow2-auto_raw-raw", absqcow2, VIR_STORAGE_FILE_AUTO,
EXP_PASS);
-
-/* qcow2 with a longer backing chain */
+/* qcow2 chain with absolute backing formatted with a real qemu-img */
TEST_CHAIN("qcow2-qcow2_qcow2-qcow2_raw-raw", abswrap,
VIR_STORAGE_FILE_QCOW2, EXP_PASS);
+TEST_CHAIN("qcow2-auto_qcow2-qcow2_raw-raw", abswrap,
VIR_STORAGE_FILE_AUTO, EXP_PASS);
/* Qcow2 file with missing backing file but specified type */
TEST_CHAIN("qcow2-qcow2_missing",
diff --git a/tests/virstoragetestdata/out/qcow2-auto_raw-raw
b/tests/virstoragetestdata/out/qcow2-auto_qcow2-qcow2_raw-raw
similarity index 74%
rename from tests/virstoragetestdata/out/qcow2-auto_raw-raw
rename to tests/virstoragetestdata/out/qcow2-auto_qcow2-qcow2_raw-raw
index 4a01b24589..2ea087592e 100644
--- a/tests/virstoragetestdata/out/qcow2-auto_raw-raw
+++ b/tests/virstoragetestdata/out/qcow2-auto_qcow2-qcow2_raw-raw
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/qcow2
+path:ABS_BUILDDIR/virstoragedata/wrap
backingStoreRaw:
capacity: 0
encryption: 0
diff --git a/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw
b/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw
deleted file mode 100644
index 57ce62a376..00
--- a/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw
+++ /dev/null
@@ -1,19 +0,0 @@
-path:ABS_BUILDDIR/virstoragedata/qcow2
-backingStoreRaw: ABS_BUILDDIR/virstoragedata/raw
-capacity: 1024
-encryption: 0
-relPath:
-type:1
-format:14
-protocol:none
-hostname:
-
-path:ABS_BUILDDIR/virstoragedata/raw
-backingStoreRaw:
-capacity: 0
-encryption: 0
-relPath:
-type:1
-format:1
-protocol:none
-hostname:
--
2.31.1
[PATCH 17/24] virstoragetest: Don't rewrite the 'qcow2' image
Create it with the appropriate backing file path rather than using another instance of 'qemu-img rebase'. Signed-off-by: Peter Krempa --- tests/virstoragetest.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 1ee7bb5230..9a7905e28d 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -132,18 +132,15 @@ testPrepImages(void) /* Create a qcow2 wrapping relative raw; later on, we modify its * metadata to test other configurations */ virCommandFree(cmd); -cmd = virCommandNewArgList(qemuimg, "create", "-f", "qcow2", NULL); -virCommandAddArgFormat(cmd, "-obacking_file=raw,backing_fmt=raw%s", - compat ? ",compat=0.10" : ""); +cmd = virCommandNewArgList(qemuimg, "create", + "-f", "qcow2", + "-F", "raw", + "-b", absraw, NULL); +if (compat) +virCommandAddArgList(cmd, "-o", "compat=0.10", NULL); virCommandAddArg(cmd, "qcow2"); if (virCommandRun(cmd, NULL) < 0) goto skip; -/* Make sure our later uses of 'qemu-img rebase' will work */ -virCommandFree(cmd); -cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2", - "-F", "raw", "-b", absraw, "qcow2", NULL); -if (virCommandRun(cmd, NULL) < 0) -goto skip; /* Create a second qcow2 wrapping the first, to be sure that we * can correctly avoid insecure probing. */ -- 2.31.1
[PATCH 16/24] virstoragetest: Stop rewriting images in 'mymain'
For testing of real images formatted by 'qemu-img' it's now sufficient
to format them once without the need to rewrtie them since we use the
real images only for testing of one scenario.
This allows us to also remove moust of the global variables holding the
path to the images which was necessary when they were being rewritten.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 33 -
1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 475ff23ce0..1ee7bb5230 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -37,28 +37,11 @@ VIR_LOG_INIT("tests.storagetest");
#define datadir abs_builddir "/virstoragedata"
-/* This test creates the following files, all in datadir:
-
- * raw: 1024-byte raw file
- * qcow2: qcow2 file with 'raw' as backing
- * wrap: qcow2 file with 'qcow2' as backing
- *
- * Relative names to these files are known at compile time, but absolute
- * names depend on where the test is run; for convenience,
- * we pre-populate the computation of these names for use during the test.
-*/
-
-static char *qemuimg;
-static char *absraw;
-static char *absqcow2;
static char *abswrap;
static void
testCleanupImages(void)
{
-VIR_FREE(qemuimg);
-VIR_FREE(absraw);
-VIR_FREE(absqcow2);
VIR_FREE(abswrap);
if (chdir(abs_builddir) < 0) {
@@ -106,8 +89,10 @@ testPrepImages(void)
bool compat = false;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *buf = NULL;
+g_autofree char *absraw = g_strdup_printf("%s/raw", datadir);
+g_autofree char *absqcow2 = g_strdup_printf("%s/qcow2", datadir);
+g_autofree char *qemuimg = virFindFileInPath("qemu-img");
-qemuimg = virFindFileInPath("qemu-img");
if (!qemuimg)
goto skip;
@@ -126,8 +111,6 @@ testPrepImages(void)
compat = true;
VIR_FREE(buf);
-absraw = g_strdup_printf("%s/raw", datadir);
-absqcow2 = g_strdup_printf("%s/qcow2", datadir);
abswrap = g_strdup_printf("%s/wrap", datadir);
if (g_mkdir_with_parents(datadir, 0777) < 0) {
@@ -158,7 +141,7 @@ testPrepImages(void)
/* Make sure our later uses of 'qemu-img rebase' will work */
virCommandFree(cmd);
cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b", "raw", "qcow2", NULL);
+ "-F", "raw", "-b", absraw, "qcow2", NULL);
if (virCommandRun(cmd, NULL) < 0)
goto skip;
@@ -469,7 +452,6 @@ mymain(void)
virStorageSource *chain = &fakeChain[0];
virStorageSource *chain2 = &fakeChain[1];
virStorageSource *chain3 = &fakeChain[2];
-g_autoptr(virCommand) cmd = NULL;
if (storageRegisterAll() < 0)
return EXIT_FAILURE;
@@ -505,13 +487,6 @@ mymain(void)
abs_srcdir
"/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2",
VIR_STORAGE_FILE_AUTO, EXP_PASS);
-/* Rewrite qcow2 file to use absolute backing name */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b", absraw, "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
/* qcow2 chain with absolute backing formatted with a real qemu-img */
TEST_CHAIN("qcow2-qcow2_qcow2-qcow2_raw-raw", abswrap,
VIR_STORAGE_FILE_QCOW2, EXP_PASS);
TEST_CHAIN("qcow2-auto_qcow2-qcow2_raw-raw", abswrap,
VIR_STORAGE_FILE_AUTO, EXP_PASS);
--
2.31.1
[PATCH 14/24] virstoragetest: Use preformatted qcow2 image for testing relative paths
More preparation for eliminating image rewriting.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 8 ++--
.../images/qcow2_raw-raw-relative.qcow2| Bin 0 -> 196616 bytes
.../out/qcow2-auto_raw-raw-relative| 2 +-
.../out/qcow2-qcow2_raw-raw-relative | 4 ++--
4 files changed, 9 insertions(+), 5 deletions(-)
create mode 100644 tests/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 15296cc14f..9af8795492 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -498,8 +498,12 @@ mymain(void)
VIR_STORAGE_FILE_AUTO, EXP_PASS);
/* Qcow2 file with relative raw backing, format provided */
-TEST_CHAIN("qcow2-qcow2_raw-raw-relative", absqcow2,
VIR_STORAGE_FILE_QCOW2, EXP_PASS);
-TEST_CHAIN("qcow2-auto_raw-raw-relative", absqcow2, VIR_STORAGE_FILE_AUTO,
EXP_PASS);
+TEST_CHAIN("qcow2-qcow2_raw-raw-relative",
+ abs_srcdir
"/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_PASS);
+TEST_CHAIN("qcow2-auto_raw-raw-relative",
+ abs_srcdir
"/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2",
+ VIR_STORAGE_FILE_AUTO, EXP_PASS);
/* Rewrite qcow2 file to use absolute backing name */
virCommandFree(cmd);
diff --git a/tests/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2
b/tests/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2
new file mode 100644
index
..492d9bca1902564ba0128fee27122f5beb0e9e32
GIT binary patch
literal 196616
zcmeIuyAi@L3;K
literal 0
HcmV?d1
diff --git a/tests/virstoragetestdata/out/qcow2-auto_raw-raw-relative
b/tests/virstoragetestdata/out/qcow2-auto_raw-raw-relative
index 4a01b24589..f9afc138f0 100644
--- a/tests/virstoragetestdata/out/qcow2-auto_raw-raw-relative
+++ b/tests/virstoragetestdata/out/qcow2-auto_raw-raw-relative
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/qcow2
+path:ABS_SRCDIR/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2
backingStoreRaw:
capacity: 0
encryption: 0
diff --git a/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw-relative
b/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw-relative
index a1fb142e98..6e3f7ab339 100644
--- a/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw-relative
+++ b/tests/virstoragetestdata/out/qcow2-qcow2_raw-raw-relative
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/qcow2
+path:ABS_SRCDIR/virstoragetestdata/images/qcow2_raw-raw-relative.qcow2
backingStoreRaw: raw
capacity: 1024
encryption: 0
@@ -8,7 +8,7 @@ format:14
protocol:none
hostname:
-path:ABS_BUILDDIR/virstoragedata/raw
+path:ABS_SRCDIR/virstoragetestdata/images/raw
backingStoreRaw:
capacity: 0
encryption: 0
--
2.31.1
[PATCH 13/24] virstoragetest: Convert symlink and relative image testing use preformatted images
Use prepared test images instead to simplify and clarify the code
instead of rewriting existing images multiple times.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 39 +++---
...2_qcow2-qcow2-symlink_raw-raw-reldir.qcow2 | Bin 0 -> 196616 bytes
.../images/qcow2_raw-raw-reldir.qcow2 | Bin 0 -> 196616 bytes
tests/virstoragetestdata/images/sub/link1 | 1 +
tests/virstoragetestdata/images/sub/link2 | 1 +
tests/virstoragetestdata/out/qcow2-symlinks | 6 +--
6 files changed, 10 insertions(+), 37 deletions(-)
create mode 100644
tests/virstoragetestdata/images/qcow2_qcow2-qcow2-symlink_raw-raw-reldir.qcow2
create mode 100644 tests/virstoragetestdata/images/qcow2_raw-raw-reldir.qcow2
create mode 12 tests/virstoragetestdata/images/sub/link1
create mode 12 tests/virstoragetestdata/images/sub/link2
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index dcb5a8a427..15296cc14f 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -42,8 +42,6 @@ VIR_LOG_INIT("tests.storagetest");
* raw: 1024-byte raw file
* qcow2: qcow2 file with 'raw' as backing
* wrap: qcow2 file with 'qcow2' as backing
- * sub/link1: symlink to qcow2
- * sub/link2: symlink to wrap
*
* Relative names to these files are known at compile time, but absolute
* names depend on where the test is run; for convenience,
@@ -54,7 +52,6 @@ static char *qemuimg;
static char *absraw;
static char *absqcow2;
static char *abswrap;
-static char *abslink2;
static void
testCleanupImages(void)
@@ -63,7 +60,6 @@ testCleanupImages(void)
VIR_FREE(absraw);
VIR_FREE(absqcow2);
VIR_FREE(abswrap);
-VIR_FREE(abslink2);
if (chdir(abs_builddir) < 0) {
fprintf(stderr, "unable to return to correct directory, refusing to "
@@ -133,10 +129,9 @@ testPrepImages(void)
absraw = g_strdup_printf("%s/raw", datadir);
absqcow2 = g_strdup_printf("%s/qcow2", datadir);
abswrap = g_strdup_printf("%s/wrap", datadir);
-abslink2 = g_strdup_printf("%s/sub/link2", datadir);
-if (g_mkdir_with_parents(datadir "/sub", 0777) < 0) {
-fprintf(stderr, "unable to create directory %s\n", datadir "/sub");
+if (g_mkdir_with_parents(datadir, 0777) < 0) {
+fprintf(stderr, "unable to create directory %s\n", datadir);
goto cleanup;
}
@@ -177,15 +172,6 @@ testPrepImages(void)
if (virCommandRun(cmd, NULL) < 0)
goto skip;
-#ifdef WITH_SYMLINK
-/* Create some symlinks in a sub-directory. */
-if (symlink("../qcow2", datadir "/sub/link1") < 0 ||
-symlink("../wrap", datadir "/sub/link2") < 0) {
-fprintf(stderr, "unable to create symlink");
-goto cleanup;
-}
-#endif
-
ret = 0;
cleanup:
if (ret)
@@ -552,25 +538,10 @@ mymain(void)
TEST_CHAIN("directory-none", abs_srcdir "/virstoragetestdata/images/",
VIR_STORAGE_FILE_NONE, EXP_PASS);
TEST_CHAIN("directory-dir", abs_srcdir "/virstoragetestdata/images/",
VIR_STORAGE_FILE_DIR, EXP_PASS);
-#ifdef WITH_SYMLINK
-/* Rewrite qcow2 and wrap file to use backing names relative to a
- * symlink from a different directory */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b", "../raw", "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "qcow2", "-b", "../sub/link1", "wrap",
- NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
/* Behavior of symlinks to qcow2 with relative backing files */
-TEST_CHAIN("qcow2-symlinks", abslink2, VIR_STORAGE_FILE_QCOW2, EXP_PASS);
-#endif
+TEST_CHAIN("qcow2-symlinks",
+ abs_srcdir "/virstoragetestdata/images/sub/link2",
+ VIR_STORAGE_FILE_QCOW2, EXP_PASS);
/* Behavior of an infinite loop chain */
TEST_CHAIN("qcow2-qcow2_infinite-self",
diff --git
a/tests/virstoragetestdata/images/qcow2_qcow2-qcow2-symlink_raw-raw-reldir.qcow2
b/tests/virstoragetestdata/images/qcow2_qcow2-qcow2-symlink_raw-raw-reldir.qcow2
new file mode 100644
index
..9620410502f21101fb64d0314cb4a0c710870dd2
GIT binary patch
literal 196616
zcmeIuJ5Iwu5CG7%104lNa7_V%Lm(m1P*FRHBP@b#97iEkoR4F03M3jfKOu@3`aE0f
z8Sjo~z3=Y&<|~9Sh$CLqDl4k4EsI&t21%Iaugk10CZl>@b$zAlE@_A|PkHla
z;dak7Y4b()>&!iW^Rsk*eleR5FQ#Snc=fB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+z&?Q_Z0>#Q<_QoWK!5-N0t5&UAV7cs
z0RjXF5FkK+0
[PATCH 12/24] virstoragetest: Use existing file for testing 'raw' image lookup
We've already added a 'raw' file to the example image directory so we
can use that instead of formatting one.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 8 ++--
tests/virstoragetestdata/out/raw-auto | 2 +-
tests/virstoragetestdata/out/raw-raw | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 34aff3e6dd..dcb5a8a427 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -504,8 +504,12 @@ mymain(void)
TEST_CHAIN("missing", "bogus", VIR_STORAGE_FILE_RAW, EXP_FAIL);
/* Raw image, whether with right format or no specified format */
-TEST_CHAIN("raw-raw", absraw, VIR_STORAGE_FILE_RAW, EXP_PASS);
-TEST_CHAIN("raw-auto", absraw, VIR_STORAGE_FILE_AUTO, EXP_PASS);
+TEST_CHAIN("raw-raw",
+ abs_srcdir "/virstoragetestdata/images/raw",
+ VIR_STORAGE_FILE_RAW, EXP_PASS);
+TEST_CHAIN("raw-auto",
+ abs_srcdir "/virstoragetestdata/images/raw",
+ VIR_STORAGE_FILE_AUTO, EXP_PASS);
/* Qcow2 file with relative raw backing, format provided */
TEST_CHAIN("qcow2-qcow2_raw-raw-relative", absqcow2,
VIR_STORAGE_FILE_QCOW2, EXP_PASS);
diff --git a/tests/virstoragetestdata/out/raw-auto
b/tests/virstoragetestdata/out/raw-auto
index 8d6c525896..d98b6e8bf5 100644
--- a/tests/virstoragetestdata/out/raw-auto
+++ b/tests/virstoragetestdata/out/raw-auto
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/raw
+path:ABS_SRCDIR/virstoragetestdata/images/raw
backingStoreRaw:
capacity: 0
encryption: 0
diff --git a/tests/virstoragetestdata/out/raw-raw
b/tests/virstoragetestdata/out/raw-raw
index 8d6c525896..d98b6e8bf5 100644
--- a/tests/virstoragetestdata/out/raw-raw
+++ b/tests/virstoragetestdata/out/raw-raw
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/raw
+path:ABS_SRCDIR/virstoragetestdata/images/raw
backingStoreRaw:
capacity: 0
encryption: 0
--
2.31.1
[PATCH 10/24] virstoragetest: Use pre-formatted file for non-path extraction test
This one doesn't require using qemu-img either.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 13 +++--
.../images/qcow2_nbd-raw.qcow2 | Bin 0 -> 196616 bytes
.../virstoragetestdata/out/qcow2-qcow2_nbd-raw | 2 +-
3 files changed, 4 insertions(+), 11 deletions(-)
create mode 100644 tests/virstoragetestdata/images/qcow2_nbd-raw.qcow2
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 96aeaef9ce..8d3dde265f 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -536,17 +536,10 @@ mymain(void)
/* Qcow2 file with missing backing file but specified type */
TEST_CHAIN("qcow2-qcow2_missing", absqcow2, VIR_STORAGE_FILE_QCOW2,
EXP_FAIL);
-
-/* Rewrite qcow2 to use an nbd: protocol as backend */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b",
"nbd+tcp://example.org:6000/blah",
- "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
/* Qcow2 file with backing protocol instead of file */
-TEST_CHAIN("qcow2-qcow2_nbd-raw", absqcow2, VIR_STORAGE_FILE_QCOW2,
EXP_PASS);
+TEST_CHAIN("qcow2-qcow2_nbd-raw",
+ abs_srcdir "/virstoragetestdata/images/qcow2_nbd-raw.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_PASS);
/* qed file */
TEST_CHAIN("qed-qed_raw",
diff --git a/tests/virstoragetestdata/images/qcow2_nbd-raw.qcow2
b/tests/virstoragetestdata/images/qcow2_nbd-raw.qcow2
new file mode 100644
index
..848da7ac9da8e115c7e2fbd334a25c1d7e6bfa28
GIT binary patch
literal 196616
zcmeIuJ&x2c6aZi+!vVMfsX<~!6cl>^BqSOt>deH;M#?0SCo6WSI3LI06i769{Zr25No>TdD#Q!f84gr-f?zFN1_C3QX)n<1t9ar)mKWqg=o>g%p*>Zz`7+b+%?
zu-FXSF~!}k9r|k8kMr4?>QKLLVpDy1Q^kHtZQRWqP=v1jy022a-3<5rG?#j;Qid$E
zt;c^Rp3f|cEL{C@=b68GT-J*3Y!Z&$0;a^2N;kBLSE2oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXFT&F;Jo!4?J0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBm#flOG7e&9Zo
literal 0
HcmV?d1
diff --git a/tests/virstoragetestdata/out/qcow2-qcow2_nbd-raw
b/tests/virstoragetestdata/out/qcow2-qcow2_nbd-raw
index 64acdb880a..08a93b9f32 100644
--- a/tests/virstoragetestdata/out/qcow2-qcow2_nbd-raw
+++ b/tests/virstoragetestdata/out/qcow2-qcow2_nbd-raw
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/qcow2
+path:ABS_SRCDIR/virstoragetestdata/images/qcow2_nbd-raw.qcow2
backingStoreRaw: nbd+tcp://example.org:6000/blah
capacity: 1024
encryption: 0
--
2.31.1
[PATCH 09/24] virstoragetest: Use a pre-formatted QED file for testing backing store extraction
The QED format isn't really being developed any more. Use a
pre-formatted image to test the existing code. In this instance we
switch to using a relative backing path for simplicity.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 21 +-
.../images/qed_raw-raw-relative | Bin 0 -> 327680 bytes
tests/virstoragetestdata/images/raw | Bin 0 -> 1024 bytes
tests/virstoragetestdata/out/qed-auto_raw | 2 +-
tests/virstoragetestdata/out/qed-qed_raw | 8 +++
5 files changed, 11 insertions(+), 20 deletions(-)
create mode 100644 tests/virstoragetestdata/images/qed_raw-raw-relative
create mode 100644 tests/virstoragetestdata/images/raw
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 3736280611..96aeaef9ce 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -42,7 +42,6 @@ VIR_LOG_INIT("tests.storagetest");
* raw: 1024-byte raw file
* qcow2: qcow2 file with 'raw' as backing
* wrap: qcow2 file with 'qcow2' as backing
- * qed: qed file with 'raw' as backing
* sub/link1: symlink to qcow2
* sub/link2: symlink to wrap
*
@@ -55,7 +54,6 @@ static char *qemuimg;
static char *absraw;
static char *absqcow2;
static char *abswrap;
-static char *absqed;
static char *abslink2;
static void
@@ -65,7 +63,6 @@ testCleanupImages(void)
VIR_FREE(absraw);
VIR_FREE(absqcow2);
VIR_FREE(abswrap);
-VIR_FREE(absqed);
VIR_FREE(abslink2);
if (chdir(abs_builddir) < 0) {
@@ -136,7 +133,6 @@ testPrepImages(void)
absraw = g_strdup_printf("%s/raw", datadir);
absqcow2 = g_strdup_printf("%s/qcow2", datadir);
abswrap = g_strdup_printf("%s/wrap", datadir);
-absqed = g_strdup_printf("%s/qed", datadir);
abslink2 = g_strdup_printf("%s/sub/link2", datadir);
if (g_mkdir_with_parents(datadir "/sub", 0777) < 0) {
@@ -181,15 +177,6 @@ testPrepImages(void)
if (virCommandRun(cmd, NULL) < 0)
goto skip;
-/* Create a qed file. */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "create", "-f", "qed", NULL);
-virCommandAddArgFormat(cmd, "-obacking_file=%s,backing_fmt=raw",
- absraw);
-virCommandAddArg(cmd, "qed");
-if (virCommandRun(cmd, NULL) < 0)
-goto skip;
-
#ifdef WITH_SYMLINK
/* Create some symlinks in a sub-directory. */
if (symlink("../qcow2", datadir "/sub/link1") < 0 ||
@@ -562,8 +549,12 @@ mymain(void)
TEST_CHAIN("qcow2-qcow2_nbd-raw", absqcow2, VIR_STORAGE_FILE_QCOW2,
EXP_PASS);
/* qed file */
-TEST_CHAIN("qed-qed_raw", absqed, VIR_STORAGE_FILE_QED, EXP_PASS);
-TEST_CHAIN("qed-auto_raw", absqed, VIR_STORAGE_FILE_AUTO, EXP_PASS);
+TEST_CHAIN("qed-qed_raw",
+ abs_srcdir "/virstoragetestdata/images/qed_raw-raw-relative",
+ VIR_STORAGE_FILE_QED, EXP_PASS);
+TEST_CHAIN("qed-auto_raw",
+ abs_srcdir "/virstoragetestdata/images/qed_raw-raw-relative",
+ VIR_STORAGE_FILE_AUTO, EXP_PASS);
/* directory */
TEST_CHAIN("directory-raw", abs_srcdir "/virstoragetestdata/images/",
VIR_STORAGE_FILE_RAW, EXP_PASS);
diff --git a/tests/virstoragetestdata/images/qed_raw-raw-relative
b/tests/virstoragetestdata/images/qed_raw-raw-relative
new file mode 100644
index
..5c91c3fcfe3dc4f25455bafada27b9c95c28c6e7
GIT binary patch
literal 327680
zcmeIuu?>JQ3cvEfb)7>21k!b^Ep^X_=py^xEHP@A-bLEY+zx
zjvg&cmMzZ
literal 0
HcmV?d1
diff --git a/tests/virstoragetestdata/images/raw
b/tests/virstoragetestdata/images/raw
new file mode 100644
index
..06d7405020018ddf3cacee90fd4af10487da3d20
GIT binary patch
literal 1024
ScmZQz7zLvtFd70QH3R?z00031
literal 0
HcmV?d1
diff --git a/tests/virstoragetestdata/out/qed-auto_raw
b/tests/virstoragetestdata/out/qed-auto_raw
index e8ab498038..292a8fa7fb 100644
--- a/tests/virstoragetestdata/out/qed-auto_raw
+++ b/tests/virstoragetestdata/out/qed-auto_raw
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/qed
+path:ABS_SRCDIR/virstoragetestdata/images/qed_raw-raw-relative
backingStoreRaw:
capacity: 0
encryption: 0
diff --git a/tests/virstoragetestdata/out/qed-qed_raw
b/tests/virstoragetestdata/out/qed-qed_raw
index 70a75c4e37..043ec4240b 100644
--- a/tests/virstoragetestdata/out/qed-qed_raw
+++ b/tests/virstoragetestdata/out/qed-qed_raw
@@ -1,5 +1,5 @@
-path:ABS_BUILDDIR/virstoragedata/qed
-backingStoreRaw: ABS_BUILDDIR/virstoragedata/raw
+path:ABS_SRCDIR/virstoragetestdata/images/qed_raw-raw-relative
+backingStoreRaw: raw
capacity: 1024
encryption: 0
relPath:
@@ -8,11 +8,11 @@ format:15
protocol:none
hostname:
-path:ABS_BUILDDIR/virstoragedata/raw
+path:ABS_SRCDIR/virstoragetestdata/images/raw
backingStoreRaw:
capacity: 0
encryption: 0
-relPath:
+relPath:raw
type:1
format:1
protocol:none
--
2.31.1
[PATCH 06/24] virstoragetest: Rework TEST_LOOKUP* cases to work on fake backing chain
Rather than using 'qemu-img' and rewriting the chain we can use fake
data and few empty files to ensure the same level of coverage. This is
possible since we've already tested that the metadata parsing from files
works properly and the only thing we are testing here is that the
symlink resolution works properly.
Additionally after the refactor of 'virstoragetest' is complete
additional tests on real data will be added.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 140 ++
tests/virstoragetestdata/lookup/qcow2 | 0
tests/virstoragetestdata/lookup/raw | 0
tests/virstoragetestdata/lookup/sub/link2 | 1 +
tests/virstoragetestdata/lookup/wrap | 0
5 files changed, 63 insertions(+), 78 deletions(-)
create mode 100644 tests/virstoragetestdata/lookup/qcow2
create mode 100644 tests/virstoragetestdata/lookup/raw
create mode 12 tests/virstoragetestdata/lookup/sub/link2
create mode 100644 tests/virstoragetestdata/lookup/wrap
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index a3f9c537e5..299b16e119 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -499,10 +499,11 @@ mymain(void)
struct testLookupData data2;
struct testPathRelativeBacking data4;
struct testBackingParseData data5;
-virStorageSource *chain2; /* short for chain->backingStore */
-virStorageSource *chain3; /* short for chain2->backingStore */
+virStorageSource fakeChain[4];
+virStorageSource *chain = &fakeChain[0];
+virStorageSource *chain2 = &fakeChain[1];
+virStorageSource *chain3 = &fakeChain[2];
g_autoptr(virCommand) cmd = NULL;
-g_autoptr(virStorageSource) chain = NULL;
if (storageRegisterAll() < 0)
return EXIT_FAILURE;
@@ -622,22 +623,29 @@ mymain(void)
/* Behavior of an infinite loop chain */
TEST_CHAIN("qcow2-qcow2_infinite-mutual", abswrap, VIR_STORAGE_FILE_QCOW2,
EXP_FAIL);
-/* Rewrite wrap and qcow2 back to 3-deep chain, absolute backing */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b", absraw, "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-/* Test behavior of chain lookups, absolute backing from relative start */
-chain = testStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2,
- -1, -1);
-if (!chain) {
-ret = -1;
+/* setup data for backing chain lookup testing */
+if (chdir(abs_srcdir "/virstoragetestdata/lookup") < 0) {
+fprintf(stderr, "unable to test relative backing chains\n");
goto cleanup;
}
-chain2 = chain->backingStore;
-chain3 = chain2->backingStore;
+
+memset(fakeChain, 0, sizeof(fakeChain));
+fakeChain[0].backingStore = &fakeChain[1];
+fakeChain[1].backingStore = &fakeChain[2];
+fakeChain[2].backingStore = &fakeChain[3];
+
+fakeChain[0].type = VIR_STORAGE_TYPE_FILE;
+fakeChain[1].type = VIR_STORAGE_TYPE_FILE;
+fakeChain[2].type = VIR_STORAGE_TYPE_FILE;
+
+fakeChain[0].format = VIR_STORAGE_FILE_QCOW2;
+fakeChain[1].format = VIR_STORAGE_FILE_QCOW2;
+fakeChain[2].format = VIR_STORAGE_FILE_RAW;
+
+/* backing chain with relative start and absolute backing paths */
+fakeChain[0].path = (char *) "wrap";
+fakeChain[1].path = (char *) abs_srcdir "/virstoragetestdata/lookup/qcow2";
+fakeChain[2].path = (char *) abs_srcdir "/virstoragetestdata/lookup/raw";
#define TEST_LOOKUP_TARGET(id, target, from, name, index, meta, parent) \
do { \
@@ -654,111 +662,87 @@ mymain(void)
TEST_LOOKUP(2, NULL, "wrap", chain, NULL);
TEST_LOOKUP(3, chain, "wrap", NULL, NULL);
TEST_LOOKUP(4, chain2, "wrap", NULL, NULL);
-TEST_LOOKUP(5, NULL, abswrap, chain, NULL);
-TEST_LOOKUP(6, chain, abswrap, NULL, NULL);
-TEST_LOOKUP(7, chain2, abswrap, NULL, NULL);
+TEST_LOOKUP(5, NULL, abs_srcdir "/virstoragetestdata/lookup/wrap", chain,
NULL);
+TEST_LOOKUP(6, chain, abs_srcdir "/virstoragetestdata/lookup/wrap", NULL,
NULL);
+TEST_LOOKUP(7, chain2, abs_srcdir "/virstoragetestdata/lookup/wrap", NULL,
NULL);
TEST_LOOKUP(8, NULL, "qcow2", chain2, chain);
TEST_LOOKUP(9, chain, "qcow2", chain2, chain);
TEST_LOOKUP(10, chain2, "qcow2", NULL, NULL);
TEST_LOOKUP(11, chain3, "qcow2", NULL, NULL);
-TEST_LOOKUP(12, NULL, absqcow2, chain2, chain);
-TEST_LOOKUP(13, chain, absqcow2, chain2, chain);
-TEST_LOOKUP(14, chain2, absqcow2, NULL, NULL);
-TEST_LOOKUP(15, chain3, absqcow2, NULL, NULL);
+TEST_LOOKUP(12, NULL, abs_srcdir "/virstoragetestdata/lookup/qcow2",
chain2, chain);
+TEST_LOOKUP(13, chain, abs_srcdir "/virstoragetestdata/lookup/qcow2",
chain2, chain);
+TEST_LOOKUP(14, chain2, abs_srcdir "/virstoragetestdata/lookup/qcow2",
NULL, NULL);
+TEST_LOOKUP(15, chain3, abs_srcdir "/virstoragetestdata/lookup/qcow
[PATCH 08/24] virstoragetest: Use existing directory in the source tree for 'directory' probing tests
We don't need a special directory for the tests. Reuse the directory
holding the data for the virstoragetest.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 13 +++--
tests/virstoragetestdata/out/directory-dir | 2 +-
tests/virstoragetestdata/out/directory-none | 2 +-
tests/virstoragetestdata/out/directory-raw | 2 +-
4 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 785699d4e8..3736280611 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -56,7 +56,6 @@ static char *absraw;
static char *absqcow2;
static char *abswrap;
static char *absqed;
-static char *absdir;
static char *abslink2;
static void
@@ -67,7 +66,6 @@ testCleanupImages(void)
VIR_FREE(absqcow2);
VIR_FREE(abswrap);
VIR_FREE(absqed);
-VIR_FREE(absdir);
VIR_FREE(abslink2);
if (chdir(abs_builddir) < 0) {
@@ -139,17 +137,12 @@ testPrepImages(void)
absqcow2 = g_strdup_printf("%s/qcow2", datadir);
abswrap = g_strdup_printf("%s/wrap", datadir);
absqed = g_strdup_printf("%s/qed", datadir);
-absdir = g_strdup_printf("%s/dir", datadir);
abslink2 = g_strdup_printf("%s/sub/link2", datadir);
if (g_mkdir_with_parents(datadir "/sub", 0777) < 0) {
fprintf(stderr, "unable to create directory %s\n", datadir "/sub");
goto cleanup;
}
-if (g_mkdir_with_parents(datadir "/dir", 0777) < 0) {
-fprintf(stderr, "unable to create directory %s\n", datadir "/dir");
-goto cleanup;
-}
if (chdir(datadir) < 0) {
fprintf(stderr, "unable to test relative backing chains\n");
@@ -573,9 +566,9 @@ mymain(void)
TEST_CHAIN("qed-auto_raw", absqed, VIR_STORAGE_FILE_AUTO, EXP_PASS);
/* directory */
-TEST_CHAIN("directory-raw", absdir, VIR_STORAGE_FILE_RAW, EXP_PASS);
-TEST_CHAIN("directory-none", absdir, VIR_STORAGE_FILE_NONE, EXP_PASS);
-TEST_CHAIN("directory-dir", absdir, VIR_STORAGE_FILE_DIR, EXP_PASS);
+TEST_CHAIN("directory-raw", abs_srcdir "/virstoragetestdata/images/",
VIR_STORAGE_FILE_RAW, EXP_PASS);
+TEST_CHAIN("directory-none", abs_srcdir "/virstoragetestdata/images/",
VIR_STORAGE_FILE_NONE, EXP_PASS);
+TEST_CHAIN("directory-dir", abs_srcdir "/virstoragetestdata/images/",
VIR_STORAGE_FILE_DIR, EXP_PASS);
#ifdef WITH_SYMLINK
/* Rewrite qcow2 and wrap file to use backing names relative to a
diff --git a/tests/virstoragetestdata/out/directory-dir
b/tests/virstoragetestdata/out/directory-dir
index cff67595b4..65b7b91912 100644
--- a/tests/virstoragetestdata/out/directory-dir
+++ b/tests/virstoragetestdata/out/directory-dir
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/dir
+path:ABS_SRCDIR/virstoragetestdata/images/
backingStoreRaw:
capacity: 0
encryption: 0
diff --git a/tests/virstoragetestdata/out/directory-none
b/tests/virstoragetestdata/out/directory-none
index cff67595b4..65b7b91912 100644
--- a/tests/virstoragetestdata/out/directory-none
+++ b/tests/virstoragetestdata/out/directory-none
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/dir
+path:ABS_SRCDIR/virstoragetestdata/images/
backingStoreRaw:
capacity: 0
encryption: 0
diff --git a/tests/virstoragetestdata/out/directory-raw
b/tests/virstoragetestdata/out/directory-raw
index ebe23cbbd6..5def2c4b8b 100644
--- a/tests/virstoragetestdata/out/directory-raw
+++ b/tests/virstoragetestdata/out/directory-raw
@@ -1,4 +1,4 @@
-path:ABS_BUILDDIR/virstoragedata/dir
+path:ABS_SRCDIR/virstoragetestdata/images/
backingStoreRaw:
capacity: 0
encryption: 0
--
2.31.1
[PATCH 07/24] virstoragetest: Test backing chain loops with hardcoded images
Provide the images for the self and mutual backing image loop cases in
the repository rather than formatting them with qemu-img.
This makes the code more readable and also decouples the backing chain
tests from each other.
Signed-off-by: Peter Krempa
---
build-aux/syntax-check.mk | 2 +-
tests/virstoragetest.c| 30 --
tests/virstoragetestdata/images/loop-1.qcow2 | Bin 0 -> 196616 bytes
tests/virstoragetestdata/images/loop-2.qcow2 | Bin 0 -> 196616 bytes
.../virstoragetestdata/images/loop-self.qcow2 | Bin 0 -> 196616 bytes
5 files changed, 7 insertions(+), 25 deletions(-)
create mode 100644 tests/virstoragetestdata/images/loop-1.qcow2
create mode 100644 tests/virstoragetestdata/images/loop-2.qcow2
create mode 100644 tests/virstoragetestdata/images/loop-self.qcow2
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 2058af0b77..cb54c8ba36 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1563,7 +1563,7 @@ exclude_file_name_regexp--sc_prohibit_close = \
(\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$)
exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \
-
(^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
+
(^tests/(nodedevmdevctl|virhostcpu|virpcitest|virstoragetest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
(^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 299b16e119..785699d4e8 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -597,31 +597,13 @@ mymain(void)
TEST_CHAIN("qcow2-symlinks", abslink2, VIR_STORAGE_FILE_QCOW2, EXP_PASS);
#endif
-/* Rewrite qcow2 to be a self-referential loop */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "qcow2", "-b", "qcow2", "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-/* Behavior of an infinite loop chain */
-TEST_CHAIN("qcow2-qcow2_infinite-self", absqcow2, VIR_STORAGE_FILE_QCOW2,
EXP_FAIL);
-
-/* Rewrite wrap and qcow2 to be mutually-referential loop */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "qcow2", "-b", "wrap", "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "qcow2", "-b", absqcow2, "wrap", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
/* Behavior of an infinite loop chain */
-TEST_CHAIN("qcow2-qcow2_infinite-mutual", abswrap, VIR_STORAGE_FILE_QCOW2,
EXP_FAIL);
+TEST_CHAIN("qcow2-qcow2_infinite-self",
+ abs_srcdir "/virstoragetestdata/images/loop-self.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_FAIL);
+TEST_CHAIN("qcow2-qcow2_infinite-mutual",
+ abs_srcdir "/virstoragetestdata/images/loop-2.qcow2",
+ VIR_STORAGE_FILE_QCOW2, EXP_FAIL);
/* setup data for backing chain lookup testing */
if (chdir(abs_srcdir "/virstoragetestdata/lookup") < 0) {
diff --git a/tests/virstoragetestdata/images/loop-1.qcow2
b/tests/virstoragetestdata/images/loop-1.qcow2
new file mode 100644
index
..21b0bb8d8749647b3de277175de0611d070b9ee8
GIT binary patch
literal 196616
zcmeIvF;2rU6aY{sEgJ(z$czL-4}pZl!o(cfAR?6{q^U}nI11^Oe5dAh-JDELcgORc5D_3ifB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FoH0
z0_lEi(=G`RAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyKwv;137_{si4*|>1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PB
[PATCH 05/24] virstoragetest: Remove redundant arguments for chain lookup tests
Passing in both "chain*" and "chain*->path" is pointless. Use only the
full struct which we can use to infer the rest.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 201 +++--
1 file changed, 94 insertions(+), 107 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 4f4fd4e824..a3f9c537e5 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -303,7 +303,6 @@ struct testLookupData
virStorageSource *from;
const char *name;
unsigned int expIndex;
-const char *expResult;
virStorageSource *expMeta;
virStorageSource *expParent;
};
@@ -318,25 +317,16 @@ testStorageLookup(const void *args)
result = virStorageSourceChainLookup(data->chain, data->from,
data->name, data->target,
&actualParent);
-if (!data->expResult)
+if (!data->expMeta)
virResetLastError();
-if (!result) {
-if (data->expResult) {
-fprintf(stderr, "result: expected %s, got NULL\n",
-data->expResult);
-ret = -1;
-}
-} else if (STRNEQ_NULLABLE(data->expResult, result->path)) {
-fprintf(stderr, "result: expected %s, got %s\n",
-NULLSTR(data->expResult), NULLSTR(result->path));
-ret = -1;
-}
if (data->expMeta != result) {
-fprintf(stderr, "meta: expected %p, got %p\n",
-data->expMeta, result);
+fprintf(stderr, "meta: expected %s, got %s\n",
+NULLSTR(data->expMeta ? data->expMeta->path : NULL),
+NULLSTR(result ? result->path : NULL));
ret = -1;
}
+
if (data->expIndex > 0) {
if (!result) {
fprintf(stderr, "index: resulting lookup is empty, can't match
index\n");
@@ -649,47 +639,44 @@ mymain(void)
chain2 = chain->backingStore;
chain3 = chain2->backingStore;
-#define TEST_LOOKUP_TARGET(id, target, from, name, index, result, \
- meta, parent) \
+#define TEST_LOOKUP_TARGET(id, target, from, name, index, meta, parent) \
do { \
data2 = (struct testLookupData){ \
-chain, target, from, name, index, \
-result, meta, parent, }; \
-if (virTestRun("Chain lookup " #id, \
- testStorageLookup, &data2) < 0) \
+chain, target, from, name, index, meta, parent, }; \
+if (virTestRun("Chain lookup " #id, testStorageLookup, &data2) < 0) \
ret = -1; \
} while (0)
-#define TEST_LOOKUP(id, from, name, result, meta, parent) \
-TEST_LOOKUP_TARGET(id, NULL, from, name, 0, result, meta, parent)
-
-TEST_LOOKUP(0, NULL, "bogus", NULL, NULL, NULL);
-TEST_LOOKUP(1, chain, "bogus", NULL, NULL, NULL);
-TEST_LOOKUP(2, NULL, "wrap", chain->path, chain, NULL);
-TEST_LOOKUP(3, chain, "wrap", NULL, NULL, NULL);
-TEST_LOOKUP(4, chain2, "wrap", NULL, NULL, NULL);
-TEST_LOOKUP(5, NULL, abswrap, chain->path, chain, NULL);
-TEST_LOOKUP(6, chain, abswrap, NULL, NULL, NULL);
-TEST_LOOKUP(7, chain2, abswrap, NULL, NULL, NULL);
-TEST_LOOKUP(8, NULL, "qcow2", chain2->path, chain2, chain);
-TEST_LOOKUP(9, chain, "qcow2", chain2->path, chain2, chain);
-TEST_LOOKUP(10, chain2, "qcow2", NULL, NULL, NULL);
-TEST_LOOKUP(11, chain3, "qcow2", NULL, NULL, NULL);
-TEST_LOOKUP(12, NULL, absqcow2, chain2->path, chain2, chain);
-TEST_LOOKUP(13, chain, absqcow2, chain2->path, chain2, chain);
-TEST_LOOKUP(14, chain2, absqcow2, NULL, NULL, NULL);
-TEST_LOOKUP(15, chain3, absqcow2, NULL, NULL, NULL);
-TEST_LOOKUP(16, NULL, "raw", chain3->path, chain3, chain2);
-TEST_LOOKUP(17, chain, "raw", chain3->path, chain3, chain2);
-TEST_LOOKUP(18, chain2, "raw", chain3->path, chain3, chain2);
-TEST_LOOKUP(19, chain3, "raw", NULL, NULL, NULL);
-TEST_LOOKUP(20, NULL, absraw, chain3->path, chain3, chain2);
-TEST_LOOKUP(21, chain, absraw, chain3->path, chain3, chain2);
-TEST_LOOKUP(22, chain2, absraw, chain3->path, chain3, chain2);
-TEST_LOOKUP(23, chain3, absraw, NULL, NULL, NULL);
-TEST_LOOKUP(24, NULL, NULL, chain3->path, chain3, chain2);
-TEST_LOOKUP(25, chain, NULL, chain3->path, chain3, chain2);
-TEST_LOOKUP(26, chain2, NULL, chain3->path, chain3, chain2);
-TEST_LOOKUP(27, chain3, NULL, NULL, NULL, NULL);
+#define TEST_LOOKUP(id, from, name, meta, parent) \
+TEST_LOOKUP_TARGET(id, NULL, from, name, 0, meta, parent)
+
+TEST_LOOKUP(0, NULL, "bogus", NULL, NULL);
+TEST_LOOKUP(1, chain, "bogus", NULL, NULL);
+TEST_LOOKUP(2, NULL, "wrap", chain, NULL);
+TEST_LOOKUP(3, chain, "wrap", NULL, NULL);
+TEST_LOOKUP(4, chain2, "wrap", NULL, NULL);
+TEST_LOOKUP(5, NULL, abswrap, chain, NULL);
+TEST_LOOKUP(6, chain, abswrap, NULL, NULL);
+TEST_LOOKUP(7, chain2, abswrap, NULL, NULL);
+TEST_LOOKUP(8, NULL, "qcow2", chain2, chain);
+TEST_LOOKUP(9, chain,
[PATCH 04/24] virstoragetest: Store output of TEST_CHAIN in output files
The TEST_CHAIN cases were storing the expected output (or rather data
to generate the expected output) in code. This made the code really hard
to follow and even harder to modify to add new cases.
This patch modifies the code to store the expected output in text files
(using the same generator as we've used to) and uses
'virTestCompareToFile' to check the outputs.
The result is that the code is way simpler and doesn't require fiddling
with 'testFileData' structs when adding new cases. Additionally this
removes mixing of code and declaration so we can stop disabling the
warning for this file.
Another advantage is that the tests are now named so it's easier to
figure out if one of them breaks.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c| 252 --
tests/virstoragetestdata/out/directory-dir| 9 +
tests/virstoragetestdata/out/directory-none | 9 +
tests/virstoragetestdata/out/directory-raw| 9 +
.../virstoragetestdata/out/qcow2-auto_raw-raw | 9 +
.../out/qcow2-auto_raw-raw-relative | 9 +
.../out/qcow2-qcow2_nbd-raw | 19 ++
.../out/qcow2-qcow2_qcow2-qcow2_raw-raw | 29 ++
.../out/qcow2-qcow2_raw-raw | 19 ++
.../out/qcow2-qcow2_raw-raw-relative | 19 ++
tests/virstoragetestdata/out/qcow2-symlinks | 29 ++
tests/virstoragetestdata/out/qed-auto_raw | 9 +
tests/virstoragetestdata/out/qed-qed_raw | 19 ++
tests/virstoragetestdata/out/raw-auto | 9 +
tests/virstoragetestdata/out/raw-raw | 9 +
15 files changed, 263 insertions(+), 195 deletions(-)
create mode 100644 tests/virstoragetestdata/out/directory-dir
create mode 100644 tests/virstoragetestdata/out/directory-none
create mode 100644 tests/virstoragetestdata/out/directory-raw
create mode 100644 tests/virstoragetestdata/out/qcow2-auto_raw-raw
create mode 100644 tests/virstoragetestdata/out/qcow2-auto_raw-raw-relative
create mode 100644 tests/virstoragetestdata/out/qcow2-qcow2_nbd-raw
create mode 100644 tests/virstoragetestdata/out/qcow2-qcow2_qcow2-qcow2_raw-raw
create mode 100644 tests/virstoragetestdata/out/qcow2-qcow2_raw-raw
create mode 100644 tests/virstoragetestdata/out/qcow2-qcow2_raw-raw-relative
create mode 100644 tests/virstoragetestdata/out/qcow2-symlinks
create mode 100644 tests/virstoragetestdata/out/qed-auto_raw
create mode 100644 tests/virstoragetestdata/out/qed-qed_raw
create mode 100644 tests/virstoragetestdata/out/raw-auto
create mode 100644 tests/virstoragetestdata/out/raw-raw
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index f6af1a17ac..4f4fd4e824 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -218,26 +218,6 @@ testPrepImages(void)
goto cleanup;
}
-/* Many fields of virStorageFileMetadata have the same content whether
- * we access the file relatively or absolutely; but file names differ
- * depending on how the chain was opened. For ease of testing, we
- * test both relative and absolute starts, and use a flag to say which
- * of the two variations to compare against. */
-typedef struct _testFileData testFileData;
-struct _testFileData
-{
-const char *expBackingStoreRaw;
-unsigned long long expCapacity;
-bool expEncrypted;
-const char *pathRel;
-const char *path;
-int type;
-int format;
-const char *secret;
-const char *hostname;
-int protocol;
-};
-
enum {
EXP_PASS = 0,
EXP_FAIL = 1,
@@ -245,33 +225,23 @@ enum {
struct testChainData
{
+const char *testname;
const char *start;
virStorageFileFormat format;
-const testFileData *files[4];
-int nfiles;
unsigned int flags;
};
-static const char testStorageChainFormat[] =
-"chain member: %zu\n"
-"path:%s\n"
-"backingStoreRaw: %s\n"
-"capacity: %lld\n"
-"encryption: %d\n"
-"relPath:%s\n"
-"type:%d\n"
-"format:%d\n"
-"protocol:%s\n"
-"hostname:%s\n";
-
static int
testStorageChain(const void *args)
{
const struct testChainData *data = args;
virStorageSource *elt;
-size_t i = 0;
g_autoptr(virStorageSource) meta = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+g_autofree char *actual = NULL;
+g_autofree char *expectpath =
g_strdup_printf("%s/virstoragetestdata/out/%s",
+ abs_srcdir, data->testname);
meta = testStorageFileGetMetadata(data->start, data->format, -1, -1);
if (!meta) {
@@ -290,47 +260,38 @@ testStorageChain(const void *args)
return -1;
}
-elt = meta;
-while (virStorageSourceIsBacking(elt)) {
-g_autofree char *expect = NULL;
-g_autofree char *actual = NULL;
+for (elt = meta; virStorageSourceIsBacking(elt); elt = elt->backingStore) {
+g_autofree char *strippedPath = virTestStablePath(elt->path);
+g_autofree char *strippedBackingStoreRaw =
virTestStablePath(elt->backingStore
[PATCH 03/24] testutils: Introduce helper for stripping bulilddir/srcdir from test outputs
In certain cases we want to be able to compare test output containing
real paths against a static output file and thus we need a helper which
strips srcdir/builddir from given path.
Signed-off-by: Peter Krempa
---
tests/testutils.c | 30 ++
tests/testutils.h | 3 +++
2 files changed, 33 insertions(+)
diff --git a/tests/testutils.c b/tests/testutils.c
index 5e9835ee89..185d281e96 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -1117,3 +1117,33 @@ const char
return virtTestCounterStr;
}
+
+
+/**
+ * virTestStablePath:
+ * @path: path to make stable
+ *
+ * If @path starts with the absolute source directory path, the prefix
+ * is replaced with the string "ABS_SRCDIR" and similarly the build directory
+ * is replaced by "ABS_BUILDDIR". This is useful when paths e.g. in output
+ * test files need to be made stable.
+ *
+ * If @path is NULL the equivalent to NULLSTR(path) is returned.
+ *
+ * The caller is responsible for freeing the returned buffer.
+ */
+char *
+virTestStablePath(const char *path)
+{
+const char *tmp;
+
+path = NULLSTR(path);
+
+if ((tmp = STRSKIP(path, abs_srcdir)))
+return g_strdup_printf("ABS_SRCDIR%s", tmp);
+
+if ((tmp = STRSKIP(path, abs_builddir)))
+return g_strdup_printf("ABS_BUILDDIR%s", tmp);
+
+return g_strdup(path);
+}
diff --git a/tests/testutils.h b/tests/testutils.h
index 48de864131..27d135fc02 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -170,3 +170,6 @@ int testCompareDomXML2XMLFiles(virCaps *caps,
bool live,
unsigned int parseFlags,
testCompareDomXML2XMLResult expectResult);
+
+char *
+virTestStablePath(const char *path);
--
2.31.1
[PATCH 01/24] virstoragetest: Drop testing of RBD backends via parsing real images
We now have specific tests for the backing store parser and previous
tests cover the extraction of the backing store string so there's no
need for these particular tests.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 38 --
1 file changed, 38 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index b80818bc7b..ab34f2f3be 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -799,44 +799,6 @@ mymain(void)
/* Behavior of an infinite loop chain */
TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_FAIL);
-/* Rewrite qcow2 to use an rbd: protocol as backend */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b", "rbd:testshare",
- "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-qcow2.expBackingStoreRaw = "rbd:testshare";
-
-/* Qcow2 file with backing protocol instead of file */
-testFileData rbd1 = {
-.path = "testshare",
-.type = VIR_STORAGE_TYPE_NETWORK,
-.format = VIR_STORAGE_FILE_RAW,
-.protocol = VIR_STORAGE_NET_PROTOCOL_RBD,
-};
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &rbd1), EXP_PASS);
-
-/* Rewrite qcow2 to use an rbd: protocol as backend */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b",
"rbd:testshare:id=asdf:mon_host=example.com",
- "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-qcow2.expBackingStoreRaw = "rbd:testshare:id=asdf:mon_host=example.com";
-
-/* Qcow2 file with backing protocol instead of file */
-testFileData rbd2 = {
-.path = "testshare",
-.type = VIR_STORAGE_TYPE_NETWORK,
-.format = VIR_STORAGE_FILE_RAW,
-.protocol = VIR_STORAGE_NET_PROTOCOL_RBD,
-.secret = "asdf",
-.hostname = "example.com",
-};
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &rbd2), EXP_PASS);
-
VIR_WARNINGS_RESET
/* Rewrite wrap and qcow2 back to 3-deep chain, absolute backing */
--
2.31.1
[PATCH 02/24] virstoragetest: Drop testing of NBD backends via parsing real images
We now have specific tests for the backing store parser and previous
tests cover the extraction of the backing store string so there's no
need for these particular tests.
Signed-off-by: Peter Krempa
---
tests/virstoragetest.c | 31 ---
1 file changed, 31 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index ab34f2f3be..f6af1a17ac 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -651,25 +651,6 @@ mymain(void)
TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_FAIL);
-/* Rewrite qcow2 to use an nbd: protocol as backend */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b",
"nbd:example.org:6000:exportname=blah",
- "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-qcow2.expBackingStoreRaw = "nbd:example.org:6000:exportname=blah";
-
-/* Qcow2 file with backing protocol instead of file */
-testFileData nbd = {
-.path = "blah",
-.type = VIR_STORAGE_TYPE_NETWORK,
-.format = VIR_STORAGE_FILE_RAW,
-.protocol = VIR_STORAGE_NET_PROTOCOL_NBD,
-.hostname = "example.org",
-};
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd), EXP_PASS);
-
/* Rewrite qcow2 to use an nbd: protocol as backend */
virCommandFree(cmd);
cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
@@ -689,18 +670,6 @@ mymain(void)
};
TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd2), EXP_PASS);
-/* Rewrite qcow2 to use an nbd: protocol without path as backend */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-F", "raw", "-b", "nbd://example.org",
- "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-qcow2.expBackingStoreRaw = "nbd://example.org";
-
-nbd2.path = NULL;
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd2), EXP_PASS);
-
/* qed file */
testFileData qed = {
.expBackingStoreRaw = absraw,
--
2.31.1
[PATCH 00/24] virstoragetest: Re-instate testing of images without backing format
Most of the series are refactors to make virstoragetest less archaic, the last commit then re-introduces testing of images which don't have backing format recorded in the metadata which can't be formatted using qemu-img any more but have security implications if we'd mishandle them. Peter Krempa (24): virstoragetest: Drop testing of RBD backends via parsing real images virstoragetest: Drop testing of NBD backends via parsing real images testutils: Introduce helper for stripping bulilddir/srcdir from test outputs virstoragetest: Store output of TEST_CHAIN in output files virstoragetest: Remove redundant arguments for chain lookup tests virstoragetest: Rework TEST_LOOKUP* cases to work on fake backing chain virstoragetest: Test backing chain loops with hardcoded images virstoragetest: Use existing directory in the source tree for 'directory' probing tests virstoragetest: Use a pre-formatted QED file for testing backing store extraction virstoragetest: Use pre-formatted file for non-path extraction test virstoragetest: Use preformatted file for testing missing backing store virstoragetest: Use existing file for testing 'raw' image lookup virstoragetest: Convert symlink and relative image testing use preformatted images virstoragetest: Use preformatted qcow2 image for testing relative paths virstoragetest: Unify testing of QCOW2 images with absolute backing virstoragetest: Stop rewriting images in 'mymain' virstoragetest: Don't rewrite the 'qcow2' image virstoragetest: Assume that 'qemu-img' supports '-o compat=' virstoragetest: testPrepImages: Don't reuse 'cmd' pointer virstoragetest: testPrepImages: Use 'qemu-img' to format 'raw' image virstoragetest: testStorageChain: Skip test if filename is NULL virstoragetest: Don't skip the whole test when qemu-img fails to format images virstoragetest: Remove pointless goto from mymain virstoragetest: Reinstate testing of images without 'backing_fmt' build-aux/syntax-check.mk | 2 +- tests/testutils.c | 30 + tests/testutils.h | 3 + tests/virstoragetest.c| 906 ++ tests/virstoragetestdata/images/loop-1.qcow2 | Bin 0 -> 196616 bytes tests/virstoragetestdata/images/loop-2.qcow2 | Bin 0 -> 196616 bytes .../virstoragetestdata/images/loop-self.qcow2 | Bin 0 -> 196616 bytes tests/virstoragetestdata/images/qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_nbd-raw.qcow2| Bin 0 -> 196616 bytes .../images/qcow2_qcow2-auto.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_qcow2-auto_qcow2-auto.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_qcow2-missing.qcow2 | Bin 0 -> 196616 bytes ...2_qcow2-qcow2-symlink_raw-raw-reldir.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_qcow2-qcow2_qcow2-auto.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_qcow2-qcow2_raw-auto.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_raw-auto.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_raw-raw-relative.qcow2 | Bin 0 -> 196616 bytes .../images/qcow2_raw-raw-reldir.qcow2 | Bin 0 -> 196616 bytes .../images/qed_raw-raw-relative | Bin 0 -> 327680 bytes tests/virstoragetestdata/images/raw | Bin 0 -> 1024 bytes tests/virstoragetestdata/images/sub/link1 | 1 + tests/virstoragetestdata/images/sub/link2 | 1 + tests/virstoragetestdata/lookup/qcow2 | 0 tests/virstoragetestdata/lookup/raw | 0 tests/virstoragetestdata/lookup/sub/link2 | 1 + tests/virstoragetestdata/lookup/wrap | 0 tests/virstoragetestdata/out/directory-dir| 9 + tests/virstoragetestdata/out/directory-none | 9 + tests/virstoragetestdata/out/directory-raw| 9 + .../out/qcow2-auto_qcow2-qcow2_raw-raw| 9 + .../out/qcow2-auto_raw-raw-relative | 9 + .../out/qcow2-qcow2_nbd-raw | 19 + .../out/qcow2-qcow2_qcow2-auto| 19 + .../out/qcow2-qcow2_qcow2-qcow2_qcow2-auto| 29 + .../out/qcow2-qcow2_qcow2-qcow2_raw-auto | 29 + .../out/qcow2-qcow2_qcow2-qcow2_raw-raw | 29 + .../out/qcow2-qcow2_raw-raw-relative | 19 + tests/virstoragetestdata/out/qcow2-symlinks | 29 + tests/virstoragetestdata/out/qed-auto_raw | 9 + tests/virstoragetestdata/out/qed-qed_raw | 19 + tests/virstoragetestdata/out/raw-auto | 9 + tests/virstoragetestdata/out/raw-raw | 9 + 42 files changed, 580 insertions(+), 628 deletions(-) create mode 100644 tests/virstoragetestdata/images/loop-1.qcow2 create mode 100644 tests/virstoragetestdata/images/loop-2.qcow2 create mode 100644 tests/virstoragetestdata/images/loop-self.qcow2 create mode 100644 tests/virstoragetestdata/images/qcow2 create mode 100644 tests/virstoragetestdata/images/qcow2_nbd-raw.qcow2 create mode 100644 tests/virstoraget
Re: [PATCH 3/9] ch_monitor: Update virCHMonitorGet to handle accept a response
On Wed, Sep 08, 2021 at 11:01:17AM -0700, William Douglas wrote:
> The virCHMonitorGet function needed to be able to return data from the
> hypervisor. This functionality is needed in order for the driver to
> support PTY enablement and getting details about the VM state.
>
> Signed-off-by: William Douglas
> ---
> src/ch/ch_monitor.c | 46 +++--
> 1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index b4bc10bfcf..44b99ef07a 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -611,12 +611,36 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char
> *endpoint)
> return ret;
> }
>
> +struct curl_data {
> +char *content;
> +size_t size;
> +};
> +
> +static size_t
> +curl_callback(void *contents, size_t size, size_t nmemb, void *userp)
> +{
> +size_t content_size = size * nmemb;
> +struct curl_data *data = (struct curl_data *)userp;
FWIW, the type cast here is redundant as 'void *' can be directly
assigned to/from any other type.
> +
> +if (content_size == 0)
> +return content_size;
> +
> +data->content = g_realloc(data->content, data->size + content_size);
> +
> +memcpy(&(data->content[data->size]), contents, content_size);
> +data->size += content_size;
> +
> +return content_size;
> +}
> +
> static int
> -virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
> +virCHMonitorGet(virCHMonitor *mon, const char *endpoint, virJSONValue
> **response)
> {
> g_autofree char *url = NULL;
> int responseCode = 0;
> int ret = -1;
> +struct curl_slist *headers = NULL;
> +struct curl_data data = {0};
>
> url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
>
> @@ -628,12 +652,30 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
> curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
> curl_easy_setopt(mon->handle, CURLOPT_URL, url);
>
> +if (response) {
> +headers = curl_slist_append(headers, "Accept: application/json");
> +headers = curl_slist_append(headers, "Content-Type:
> application/json");
> +curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
> +curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION, curl_callback);
> +curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data);
> +}
> +
> responseCode = virCHMonitorCurlPerform(mon->handle);
>
> virObjectUnlock(mon);
>
> -if (responseCode == 200 || responseCode == 204)
> +if (responseCode == 200 || responseCode == 204) {
> ret = 0;
> +if (response) {
> +data.content = g_realloc(data.content, data.size + 1);
> +data.content[data.size] = 0;
> +*response = virJSONValueFromString(data.content);
Oh, well need to add:
if (!*response)
return -1;
in case JSON parsing fails
> +}
> +}
> +
> +g_free(data.content);
> +/* reset the libcurl handle to avoid leaking a stack pointer to data */
> +curl_easy_reset(mon->handle);
>
> return ret;
> }
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
