On 03/02/2018 02:22 PM, John Ferlan wrote:
>
>
> On 02/21/2018 01:11 PM, Michal Privoznik wrote:
>> This is the easier part. All we need to do here is put -object
>> pr-manger-helper,id=$alias,path=$socketPath and then just
>> reference the object in -drive file.pr-manger=$alias.
>
> s/manger/manager/
>
> My fingers usually the same way though as manger
>
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_command.c | 40 ++++++++++++++++++++++
>> .../disk-virtio-scsi-reservations-not-managed.args | 28 +++++++++++++++
>> .../disk-virtio-scsi-reservations.args | 29 ++++++++++++++++
>> tests/qemuxml2argvtest.c | 8 +++++
>> 4 files changed, 105 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args
>> create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index fa0aa5d5c..069d60d35 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1514,6 +1514,20 @@ qemuDiskSourceGetProps(virStorageSourcePtr src)
>> }
>>
>>
>> +static void
>> +qemuBuildDriveSourcePR(virBufferPtr buf,
>> + virStorageSourcePtr src)
>> +{
>> + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
>> + qemuDomainDiskPRDPtr prd = srcPriv->prd;
>> +
>> + if (!prd || !prd->alias)
>> + return;
>> +
>> + virBufferAsprintf(buf, ",file.pr-manager=%s", prd->alias);
>> +}
>> +
>> +
>> static int
>> qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>> virQEMUCapsPtr qemuCaps,
>> @@ -1590,6 +1604,8 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>>
>> if (disk->src->debug)
>> virBufferAsprintf(buf, ",file.debug=%d", disk->src->debugLevel);
>> +
>> + qemuBuildDriveSourcePR(buf, disk->src);
>> } else {
>> if (!(source = virQEMUBuildDriveCommandlineFromJSON(srcprops)))
>> goto cleanup;
>> @@ -9789,6 +9805,28 @@ qemuBuildPanicCommandLine(virCommandPtr cmd,
>> }
>>
>>
>> +static void
>> +qemuBuildMasterPRCommandLine(virCommandPtr cmd,
>> + const virDomainDef *def)
>> +{
>> + size_t i;
>> +
>> + for (i = 0; i < def->ndisks; i++) {
>> + const virDomainDiskDef *disk = def->disks[i];
>> + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
>> + qemuDomainDiskPRDPtr prd = srcPriv->prd;
>> + virBuffer buf = VIR_BUFFER_INITIALIZER;
>> +
>> + if (!prd || !prd->alias)
>> + continue;
>> +
>> + virBufferAsprintf(&buf, "pr-manager-helper,id=%s,path=%s", prd->alias, prd->path);
>> + virCommandAddArg(cmd, "-object");
>> + virCommandAddArgBuffer(cmd, &buf);
>
> What happens when there's more than one disk using the managed mode
> where you have a "static" alias and path, wouldn't there be multiple
> lines with:
>
> -object
> pr-manager-helper,id=pr-helper0,path=/tmp/lib/domain--1-QEMUGuest1/pr-helper0.sock
Ah sure.
>
> ? And how is QEMU going to react to that?
>
> IOW: Shouldn't this code know it's already created an object for that
> case and not generate another one?
>
> The other one :
>
> -object pr-manager-helper,id=pr-helper-sdb,path=/path/to/qemu-pr-helper.sock
>
> I get, but I'm still not thrilled with "sdb" as opposed to the
> disk->info.alias of "scsi0-0-0-0" more commonly used. IIRC, there's no
> guarantee that what libvirt calls "sdb" ends up being "sdb" on the
> guest. My vague recollection of the algorithm that "automagically"
> generates the address based on sda, sdb, sdc, etc. So "sdb" IIRC would
> related to an address that would create an alias using "0-0-1"; whereas,
> "sda" would create that "0-0-0" value.
Yes. I've changed it in the other patch so now 'pr-helper-scsi0-0-0-0'
is generated.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
