Re: [libvirt] [PATCH 2/2] qemu: Don't fail if the SCSI host device is shareable between domains
On 01/16/2014 08:53 AM, Osier Yang wrote:
<...snip...>
> It took some thinking, but I convinced myself that this path doesn't
> need the shared check since it's only called from qemuProcessReconnect;
> however, if something else did call it some day then that check may be
> necessary. It may be wise to add it anyway... I have no strong opinion
> about it being required for this change.
>
Missed reply for this one.
Actually it needs the checking. Assuming there are 2 live domains, and
they are using the same SCSI generic device. It's possible since the
device could be shared among domains. And in that case, we should
update the "dev->used_by" array instead of adding the device to the list
("driver->activeScsiHostdevs") directly, during the reconnecting sequence.
I.E it needs to restore the internal data correctly to the state just as the
one before libvirtd getting shutdown.
The activeScsiHostdevs is listed off the driver and not the domain, so
it's not like each domain has to update which other domains is sharing.
Maybe I'm misreading your thoughts regarding needing to update instead
of add.
If there were 2..n already running with the share attribute already set,
would the reconnect threads run/finish before the new libvirtd accepts
new connections/domain starts? If so, then this code will restore life
as it was before libvirtd was stopped. The only way those domains would
have been able to start previously and share the device is if they ran
through the qemuPrepareHostdevSCSIDevices() which does check shareable.
My assumption was after some amount of code reading - those restart
threads would finish so that it's not possible for some domain to be
started that isn't sharing, but wanting to use the same device.
Since prior code doesn't allow sharing of these devices there doesn't
need to be a consideration made for an already running domain when this
code first appears. Although, you may run into situations where
qemuPrepareHostdevSCSIDevices() fails due to a running domain that was
running before the shareable attribute was known. That domain would have
to be restarted - something that could be documentable. The error
message could be more helpful indicating it's in use without the
shareable attribute by some other domain. In that case, only 1 domain
would be using it thus in theory used_by[0] would be that domain. If
there were more than 1 domain (n_used_by), then you've got other
problems in the code!
John