[libvirt] [PATCH v2 04/12] qemu: Generate alias and socket path for pr-helper

Michal Privoznik mprivozn at redhat.com
Tue Mar 6 17:31:48 UTC 2018


On 03/02/2018 02:59 AM, John Ferlan wrote:
> 
> 
> On 02/21/2018 01:11 PM, Michal Privoznik wrote:
>> While we're not generating the command line just yet (look for
>> the next commits), 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 at redhat.com>
>> ---
>>  src/libvirt_private.syms  |  2 ++
>>  src/qemu/qemu_domain.c    | 80 +++++++++++++++++++++++++++++++++++++++++++++--
>>  src/qemu/qemu_domain.h    | 10 ++++++
>>  src/util/virstoragefile.c | 15 +++++++++
>>  src/util/virstoragefile.h |  2 ++
>>  5 files changed, 106 insertions(+), 3 deletions(-)
>>

>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index de8974d66..dffc4c30e 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -11538,17 +11548,81 @@ qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver,
>>  }
>>  
>>  
>> +static int
>> +qemuDomainPrepareDiskPRD(qemuDomainObjPrivatePtr priv,
>> +                         virDomainDiskDefPtr disk)
>> +{
>> +    qemuDomainStorageSourcePrivatePtr srcPriv;
>> +    virStoragePRDefPtr prd = disk->src->pr;
>> +    char *prAlias = NULL;
>> +    char *prPath = NULL;
>> +    int ret = -1;
>> +
>> +    if (!virStoragePRDefIsEnabled(prd))
>> +        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 (!virStorageSourceIsLocalStorage(disk->src)) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                       _("reservations supported only for local storage"));
>> +        goto cleanup;
>> +    }
> 
> Ironically the above two could return -1 directly...
> 
>> +
>> +    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
>> +
>> +    if (virStoragePRDefIsManaged(prd)) {
>> +        /* Generate PR helper socket path & alias that are same
>> +         * for each disk in the domain. */
>> +
>> +        if (VIR_STRDUP(prAlias, "pr-helper0") < 0)
>> +            return -1;
>> +
>> +        if (virAsprintf(&prPath, "%s/pr-helper0.sock", priv->libDir) < 0)
>> +            return -1;
> 
> Leaks prAlias

Ah, good catch.

> 
> BTW: Whatever "managed" ends up being needs to be described somewhere
> either prior to this or after this, but knowing how the connection
> between libvirt and qemu's process is going to happen will be really
> helpful... Something that I would think would be described in patch 1,
> but again that's just me.  This whole managed thing is really confusing
> the way it's written. May make sense to you, but it doesn't make sense
> to me, just saying.

Fair enough. I'll add something.

> 
>> +
>> +    } else {
>> +        if (virAsprintf(&prAlias, "pr-helper-%s", disk->dst) < 0)
>> +            return -1;
> 
> Or using the 'disk->info.alias' like other consumers.

Okay. I though that disk->dst would be easier to find on cmd line but on
the other hand, grepping cmd line for disk->info.alias shows every
argument that has something to do with the disk.

Michal




More information about the libvir-list mailing list