On Thu, Jan 18, 2018 at 17:04:37 +0100, Michal Privoznik wrote:
> While we're not generating the command line just yet (look for
> the next commit), we can generate the alias for pr-manager.
> A domain can have up to one managed pr-manager (in which case
> socket path is decided by libvirt and pr-helper is spawned by
> libvirt too), but up to ndisks of unmanaged ones (one per disk
> basically). In case of the former we can generate a simple alias
> and be sure it'll not conflict. But in case of the latter to
> avoid any conflicts let's generate alias that's based on
> something unique - like disk target.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_domain.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 3 +++
> 2 files changed, 64 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 7fa8c93b7..e8d2adf56 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -972,6 +972,8 @@ qemuDomainStorageSourcePrivateDispose(void *obj)
>
> qemuDomainSecretInfoFree(&priv->secinfo);
> qemuDomainSecretInfoFree(&priv->encinfo);
> +
> + VIR_FREE(priv->prAlias);
> }
>
>
> @@ -11037,6 +11039,62 @@ qemuDomainDiskPRObjectKillAll(qemuDomainObjPrivatePtr priv)
> }
>
>
> +static int
> +qemuDomainPrepareDiskPR(qemuDomainObjPrivatePtr priv,
> + virDomainDiskDefPtr disk)
> +{
> + qemuDomainStorageSourcePrivatePtr srcPriv;
> + virStoragePRDefPtr prd = disk->src->pr;
> + char *prPath = NULL;
> + bool managed;
> + int ret = -1;
> +
> + if (!prd ||
> + prd->enabled != VIR_TRISTATE_BOOL_YES)
> + return 0;
> +
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("reservations not supported with this QEMU binary"));
> + goto cleanup;
> + }
> +
> + if (!disk->src->privateData &&
> + !(disk->src->privateData = qemuDomainStorageSourcePrivateNew()))
> + return -1;
> +
> + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> + managed = prd->managed == VIR_TRISTATE_BOOL_YES;
> +
> + if (managed) {
> + /* Generate PR helper socket path & alias that are same
> + * for each disk in the domain. */
> +
> + if (VIR_STRDUP(srcPriv->prAlias, "pr-helper0") < 0)
> + return -1;
> +
> + if (virAsprintf(&prPath, "%s/pr-helper0.sock", priv->libDir) < 0)
> + return -1;
> +
> + } else {
> + if (virAsprintf(&srcPriv->prAlias, "pr-helper-%s", disk->dst) < 0)
> + return -1;
I though that unmanaged PRs would be shared which would make sense given
the data structure you've introduced in patch 4. Since this code would
in that case make problems with hotplug I looked back at that code.
So, this means that there's exactly one managed PR daemon and every
unmanaged PR daemon has possibly multiple instances/connections from
qemu.
So, that brings me to the question: Do you really need the prHelpers
hash table? If the instances are duplicated, you can store the data in
virStorageSource (in the private data) and don't really need the table.
If there's intent to add sharing of the pr objects in qemu, you'll need
to rething this code, since generating the alias will create problems
when you hotunplug a shared PR instance and try to plug back a disk with
a different one.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
