On 2012年12月15日 06:05, Eric Blake wrote:
On 12/14/2012 08:45 AM, Osier Yang wrote:
Just like for domain starting, this sets the disk's unpriv_sgio if cdbfilter is specified when attaching disk. --- src/qemu/qemu_driver.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-)
+ + /* Set sysfs unpriv_sgio if cdbfilter is specified. */ + if (disk->cdbfilter) { + if (virSetDeviceUnprivSGIO(disk->src, NULL, + (disk->cdbfilter == + VIR_DOMAIN_DISK_CDB_FILTER_NO) + ? 1 : 0)< 0) { + VIR_WARN("Failed to set unpriv_sgio of disk '%s'", disk->src); + goto end; + } + }
Again, does this do the right thing if some other domain already had cdbfilter='no' (aka kernel allows SG_IO) but this domain omitted the cdbfilter attribute (which defaults to cdbfilter='yes' but does not satisfy the if(disk->cdbfilter) condition)?
I think it does the right thing, because not specifying "cdbfilter" should mean it doesn't care about what the kernel setting is, not means cdbfiler="yes" instead. Assuming that one used rawio="yes" for a domain, and if we defaults cdbfilter to "yes", it will cause regression, as the user still expects SG_IO commands can be executed on the disk, however, it's disabled by cdbfilter="yes".
I suspect that will be worked out when you rework things to use a tri-state rawio.
Of course, this is based on we use a separate XML tag like cdbfilter, it won't stand anymore if we finally use a tri-state rawio. Regards, Osier