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.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
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);
+ }
+}
+
+
/**
* qemuBuildCommandLineValidate:
*
@@ -9941,6 +9979,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0)
goto error;
+ qemuBuildMasterPRCommandLine(cmd, def);
+
if (enableFips)
virCommandAddArg(cmd, "-enable-fips");
diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args
new file mode 100644
index 000000000..387432c18
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args
@@ -0,0 +1,28 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-object pr-manager-helper,id=pr-helper-sdb,path=/path/to/qemu-pr-helper.sock \
+-M pc \
+-m 214 \
+-smp 8,sockets=8,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,file.pr-manager=pr-helper-sdb,format=raw,\
+if=none,id=drive-scsi0-0-0-0,boot=on \
+-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
new file mode 100644
index 000000000..2dc53cf05
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
@@ -0,0 +1,29 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-object pr-manager-helper,id=pr-helper0,\
+path=/tmp/lib/domain--1-QEMUGuest1/pr-helper0.sock \
+-M pc \
+-m 214 \
+-smp 8,sockets=8,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,file.pr-manager=pr-helper0,format=raw,\
+if=none,id=drive-scsi0-0-0-0,boot=on \
+-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 688846b9b..429cdf079 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2966,6 +2966,14 @@ mymain(void)
QEMU_CAPS_HDA_DUPLEX);
DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI);
+ DO_TEST("disk-virtio-scsi-reservations",
+ QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER);
+
+ DO_TEST("disk-virtio-scsi-reservations-not-managed",
+ QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER);
+
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
--
2.16.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com> > --- > 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 ? 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. The fact that you've defined the <address> and <target> originally avoids the virDomainDiskDefAssignAddress algorithm. Also see the virDiskNameToBusDeviceIndex as well as the virDomainHostdevAssignAddress (and code around that) in order to see the awfulness of which I write. The real fun begins when you have <disk>'s and <hostdev>'s. BTW: Seeing this and thinking about the command line jiggles a memory thread related to virStorageSourceParseBacking* and related tests where the code can handle various JSON outputs where it's not clear to me whether you'll need to add tests for the "file.pr-manager" processing. I think you might, but Peter understands more of that than I do. John > + } > +} > + > + > /** > * qemuBuildCommandLineValidate: > * > @@ -9941,6 +9979,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, > if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) > goto error; > > + qemuBuildMasterPRCommandLine(cmd, def); > + > if (enableFips) > virCommandAddArg(cmd, "-enable-fips"); > > diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args > new file mode 100644 > index 000000000..387432c18 > --- /dev/null > +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args > @@ -0,0 +1,28 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/home/test \ > +USER=test \ > +LOGNAME=test \ > +QEMU_AUDIO_DRV=none \ > +/usr/bin/qemu-system-i686 \ > +-name QEMUGuest1 \ > +-S \ > +-object pr-manager-helper,id=pr-helper-sdb,path=/path/to/qemu-pr-helper.sock \ > +-M pc \ > +-m 214 \ > +-smp 8,sockets=8,cores=1,threads=1 \ > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > +-nographic \ > +-nodefaults \ > +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ > +server,nowait \ > +-mon chardev=charmonitor,id=monitor,mode=readline \ > +-no-acpi \ > +-boot c \ > +-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \ > +-usb \ > +-drive file=/dev/HostVG/QEMUGuest1,file.pr-manager=pr-helper-sdb,format=raw,\ > +if=none,id=drive-scsi0-0-0-0,boot=on \ > +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ > +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ > +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 > diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args > new file mode 100644 > index 000000000..2dc53cf05 > --- /dev/null > +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args > @@ -0,0 +1,29 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/home/test \ > +USER=test \ > +LOGNAME=test \ > +QEMU_AUDIO_DRV=none \ > +/usr/bin/qemu-system-i686 \ > +-name QEMUGuest1 \ > +-S \ > +-object pr-manager-helper,id=pr-helper0,\ > +path=/tmp/lib/domain--1-QEMUGuest1/pr-helper0.sock \ > +-M pc \ > +-m 214 \ > +-smp 8,sockets=8,cores=1,threads=1 \ > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > +-nographic \ > +-nodefaults \ > +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ > +server,nowait \ > +-mon chardev=charmonitor,id=monitor,mode=readline \ > +-no-acpi \ > +-boot c \ > +-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \ > +-usb \ > +-drive file=/dev/HostVG/QEMUGuest1,file.pr-manager=pr-helper0,format=raw,\ > +if=none,id=drive-scsi0-0-0-0,boot=on \ > +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ > +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ > +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 688846b9b..429cdf079 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -2966,6 +2966,14 @@ mymain(void) > QEMU_CAPS_HDA_DUPLEX); > DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); > > + DO_TEST("disk-virtio-scsi-reservations", > + QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI, > + QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER); > + > + DO_TEST("disk-virtio-scsi-reservations-not-managed", > + QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI, > + QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER); > + > if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) > virFileDeleteTree(fakerootdir); > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Mar 02, 2018 at 08:22:32 -0500, 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@redhat.com> > > --- > > 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 > > ? 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. > > The fact that you've defined the <address> and <target> originally > avoids the virDomainDiskDefAssignAddress algorithm. Also see the > virDiskNameToBusDeviceIndex as well as the virDomainHostdevAssignAddress > (and code around that) in order to see the awfulness of which I write. > The real fun begins when you have <disk>'s and <hostdev>'s. > > BTW: Seeing this and thinking about the command line jiggles a memory > thread related to virStorageSourceParseBacking* and related tests where > the code can handle various JSON outputs where it's not clear to me > whether you'll need to add tests for the "file.pr-manager" processing. I > think you might, but Peter understands more of that than I do. No, this should not be handled in JSON at all. Referencing an alias in JSON is wrong since it is tied to the one single run of the VM where that would be created and could be something completely different in any other run of the VM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com> >> --- >> 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/06/2018 12:31 PM, Michal Privoznik wrote: > 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@redhat.com> >>> --- >>> 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. > So having multiple "id=pr-helper0" is OK by QEMU? OK, then. John >> >> ? 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/08/2018 01:18 AM, John Ferlan wrote: > > > On 03/06/2018 12:31 PM, Michal Privoznik wrote: >> On 03/02/2018 02:22 PM, John Ferlan wrote: >>> >>> >>> On 02/21/2018 01:11 PM, Michal Privoznik wrote: >>> 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. >> > > So having multiple "id=pr-helper0" is OK by QEMU? OK, then. No it's not. I've fixed this locally and I'll post v3 soon. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.