subject:"\[libvirt\] \[PATCH 2\/3\] qemu\: Process the hostdev rawio setting"

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

2014-09-19 Thread Roman Bogorodskiy

  John Ferlan wrote:

> 
> 
> On 09/18/2014 12:50 PM, Michal Privoznik wrote:
> > On 10.09.2014 01:40, John Ferlan wrote:
> >> Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev
> >> lun device.
> <...snip...>
> 
> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> >> index b1d8a32..3544716 100644
> >> --- a/src/qemu/qemu_process.c
> >> +++ b/src/qemu/qemu_process.c
> >> @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
> >>   struct qemuProcessHookData hookData;
> >>   unsigned long cur_balloon;
> >>   size_t i;
> >> +bool rawio_set = false;
> >>   char *nodeset = NULL;
> >>   virBitmapPtr nodemask = NULL;
> >>   unsigned int stop_flags;
> >> @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
> >>   virDomainDeviceDef dev;
> >>   virDomainDiskDefPtr disk = vm->def->disks[i];
> >>
> >> -if (vm->def->disks[i]->rawio == 1)
> >> +if (vm->def->disks[i]->rawio == 1) {
> >>   #ifdef CAP_SYS_RAWIO
> >>   virCommandAllowCap(cmd, CAP_SYS_RAWIO);
> >>   #else
> >>   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> >>  _("Raw I/O is not supported on this 
> >> platform"));
> >>   #endif
> >> +rawio_set = true;
> >> +}
> > 
> > Interesting. So if rawio is requested we shout an error but don't fail 
> > actually. I think we are missing 'goto cleanup' here.
> > 
> 
> See 9a2f36ec04e0436b1ba9f0c21f9be234b25ac579
> 
> I can add a goto if desired or perhaps change it to a VIR_WARN() or
> something else well.
> 
> I've copied the author of that commit to get an opinion...

Hi,

Yes, I guess 'goto cleanup' is missing here indeed. VIR_WARN() will also
work, but probably it'd be better user experience to report an unsupported
configuration and fail early instead of showing a warning that could be
missed.

Roman Bogorodskiy

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

2014-09-18 Thread John Ferlan



On 09/18/2014 12:50 PM, Michal Privoznik wrote:
> On 10.09.2014 01:40, John Ferlan wrote:
>> Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev
>> lun device.
<...snip...>

>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index b1d8a32..3544716 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
>>   struct qemuProcessHookData hookData;
>>   unsigned long cur_balloon;
>>   size_t i;
>> +bool rawio_set = false;
>>   char *nodeset = NULL;
>>   virBitmapPtr nodemask = NULL;
>>   unsigned int stop_flags;
>> @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
>>   virDomainDeviceDef dev;
>>   virDomainDiskDefPtr disk = vm->def->disks[i];
>>
>> -if (vm->def->disks[i]->rawio == 1)
>> +if (vm->def->disks[i]->rawio == 1) {
>>   #ifdef CAP_SYS_RAWIO
>>   virCommandAllowCap(cmd, CAP_SYS_RAWIO);
>>   #else
>>   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>  _("Raw I/O is not supported on this platform"));
>>   #endif
>> +rawio_set = true;
>> +}
> 
> Interesting. So if rawio is requested we shout an error but don't fail 
> actually. I think we are missing 'goto cleanup' here.
> 

See 9a2f36ec04e0436b1ba9f0c21f9be234b25ac579

I can add a goto if desired or perhaps change it to a VIR_WARN() or
something else well.

I've copied the author of that commit to get an opinion...


>>
>>   dev.type = VIR_DOMAIN_DEVICE_DISK;
>>   dev.data.disk = disk;
>> @@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn,
>>   goto cleanup;
>>   }
>>
>> +/* If rawio not already set, check hostdevs as well */
>> +if (!rawio_set) {
>> +for (i = 0; i < vm->def->nhostdevs; i++) {
>> +virDomainHostdevSubsysSCSIPtr scsisrc =
>> +&vm->def->hostdevs[i]->source.subsys.u.scsi;
>> +if (scsisrc->rawio == 1)
>> +#ifdef CAP_SYS_RAWIO
>> +virCommandAllowCap(cmd, CAP_SYS_RAWIO);
>> +#else
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +   _("Raw I/O is not supported on this 
>> platform"));
>> +#endif
> 
> And here too then.
> 

Monkey see, monkey do[o] ;-)

John

>> +}
>> +}
>> +

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

2014-09-18 Thread Michal Privoznik


On 10.09.2014 01:40, John Ferlan wrote:

Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev
lun device.

Signed-off-by: John Ferlan 
---
  src/qemu/qemu_domain.c  | 21 +
  src/qemu/qemu_domain.h  |  4 
  src/qemu/qemu_driver.c  |  1 +
  src/qemu/qemu_process.c | 20 +++-
  4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 306ff10..166fadb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1715,6 +1715,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
  for (i = 0; i < obj->def->ndisks; i++)
  qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logFD);

+for (i = 0; i < obj->def->nhostdevs; i++)
+qemuDomainObjCheckHostdevTaint(driver, obj, obj->def->hostdevs[i],
+   logFD);
+
  for (i = 0; i < obj->def->nnets; i++)
  qemuDomainObjCheckNetTaint(driver, obj, obj->def->nets[i], logFD);

@@ -1741,6 +1745,23 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
  }


+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr hostdev,
+int logFD)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);


This @cfg is not used anywhere in the function. Well, technically is - 
it's unref'd but besides that, it isn't. Is it a leftover from previous 
code of yours that you were testing?



+virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+
+if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
+scsisrc->rawio == 1)


If you follow my advice from 1/3 then this would be 
s/1/VIR_TRISTATE_BOOL_YES/



+qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
+   logFD);
+
+virObjectUnref(cfg);
+}
+
+
  void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
  virDomainObjPtr obj,
  virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f353d90..7aebb0f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -286,6 +286,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
   virDomainObjPtr obj,
   virDomainDiskDefPtr disk,
   int logFD);
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr disk,
+int logFD);
  void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
  virDomainObjPtr obj,
  virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d724eeb..78ecb3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6548,6 +6548,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
  break;

  case VIR_DOMAIN_DEVICE_HOSTDEV:
+qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1);
  ret = qemuDomainAttachHostDevice(dom->conn, driver, vm,
   dev->data.hostdev);
  if (!ret)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1d8a32..3544716 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
  struct qemuProcessHookData hookData;
  unsigned long cur_balloon;
  size_t i;
+bool rawio_set = false;
  char *nodeset = NULL;
  virBitmapPtr nodemask = NULL;
  unsigned int stop_flags;
@@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
  virDomainDeviceDef dev;
  virDomainDiskDefPtr disk = vm->def->disks[i];

-if (vm->def->disks[i]->rawio == 1)
+if (vm->def->disks[i]->rawio == 1) {
  #ifdef CAP_SYS_RAWIO
  virCommandAllowCap(cmd, CAP_SYS_RAWIO);
  #else
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
 _("Raw I/O is not supported on this platform"));
  #endif
+rawio_set = true;
+}


Interesting. So if rawio is requested we shout an error but don't fail 
actually. I think we are missing 'goto cleanup' here.




  dev.type = VIR_DOMAIN_DEVICE_DISK;
  dev.data.disk = disk;
@@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn,
  goto cleanup;
  }

+/* If rawio not already set, check hostdevs as well */
+if (!rawio_set) {
+for (i = 0; i < vm->def->nhostdevs; i++) {
+virDomainHostdevSubsysSCSIPtr scsisrc =
+&vm->def->hostdevs[i]->s

[libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

2014-09-09 Thread John Ferlan

Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev
lun device.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_domain.c  | 21 +
 src/qemu/qemu_domain.h  |  4 
 src/qemu/qemu_driver.c  |  1 +
 src/qemu/qemu_process.c | 20 +++-
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 306ff10..166fadb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1715,6 +1715,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
 for (i = 0; i < obj->def->ndisks; i++)
 qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logFD);
 
+for (i = 0; i < obj->def->nhostdevs; i++)
+qemuDomainObjCheckHostdevTaint(driver, obj, obj->def->hostdevs[i],
+   logFD);
+
 for (i = 0; i < obj->def->nnets; i++)
 qemuDomainObjCheckNetTaint(driver, obj, obj->def->nets[i], logFD);
 
@@ -1741,6 +1745,23 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
 }
 
 
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr hostdev,
+int logFD)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+
+if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
+scsisrc->rawio == 1)
+qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
+   logFD);
+
+virObjectUnref(cfg);
+}
+
+
 void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
 virDomainObjPtr obj,
 virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f353d90..7aebb0f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -286,6 +286,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
  virDomainObjPtr obj,
  virDomainDiskDefPtr disk,
  int logFD);
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr disk,
+int logFD);
 void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
 virDomainObjPtr obj,
 virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d724eeb..78ecb3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6548,6 +6548,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 break;
 
 case VIR_DOMAIN_DEVICE_HOSTDEV:
+qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1);
 ret = qemuDomainAttachHostDevice(dom->conn, driver, vm,
  dev->data.hostdev);
 if (!ret)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1d8a32..3544716 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
 struct qemuProcessHookData hookData;
 unsigned long cur_balloon;
 size_t i;
+bool rawio_set = false;
 char *nodeset = NULL;
 virBitmapPtr nodemask = NULL;
 unsigned int stop_flags;
@@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
 virDomainDeviceDef dev;
 virDomainDiskDefPtr disk = vm->def->disks[i];
 
-if (vm->def->disks[i]->rawio == 1)
+if (vm->def->disks[i]->rawio == 1) {
 #ifdef CAP_SYS_RAWIO
 virCommandAllowCap(cmd, CAP_SYS_RAWIO);
 #else
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Raw I/O is not supported on this platform"));
 #endif
+rawio_set = true;
+}
 
 dev.type = VIR_DOMAIN_DEVICE_DISK;
 dev.data.disk = disk;
@@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn,
 goto cleanup;
 }
 
+/* If rawio not already set, check hostdevs as well */
+if (!rawio_set) {
+for (i = 0; i < vm->def->nhostdevs; i++) {
+virDomainHostdevSubsysSCSIPtr scsisrc =
+&vm->def->hostdevs[i]->source.subsys.u.scsi;
+if (scsisrc->rawio == 1)
+#ifdef CAP_SYS_RAWIO
+virCommandAllowCap(cmd, CAP_SYS_RAWIO);
+#else
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Raw I/O is not supported on this platform"));
+#endif
+}
+}
+
 virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData);
 virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
 virCommandSetMaxFiles(cmd, cfg->ma

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

[libvirt] [PATCH 2/3] qemu: Process the hostdev "rawio" setting

4 matches

Site Navigation

Mail list logo

Footer information