Re: [libvirt] [PATCH v3 03/10] Introduce a "scsi_host" hostdev type
[...]
> I don't think the current code naming is incorrect, but it
does
> slightly paint us into a box with this work. I'll mull this over
> overnight, and maybe cook up a cleanup patch separate from this
> series. Or perhaps take your other suggestion and go with the
> inclusion of "vhost" in the functions.
John,
I sent an RFC patch [1] separate from this series the other day, but
thought that I had the remainder of your comments addressed and so maybe
I'd combine everything into one series. Then my brain exploded:
Before:
// These three are all existing code
virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
// The next one is new
virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host;
After:
// These three are all existing code
virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
virDomainHostdevSubsysSCSISCSIHostPtr scsiscsihostsrc =
&scsisrc->u.host;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
// The next one is new
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&def->source.subsys.u.scsihost;
So, uh, ugh. (And it still has an inconsistency because I had to
prepend another "scsi" on the existing "scsihostsrc" ... which means
there's probably a better reworking to have happen here.)
I could take your other suggestion of "SCSIHostVHost", but I still worry
that that gets viewed as a subset of the existing SCSIHost stuff (which
is a type='scsi' sourceadapter='scsi_host' hostdev), without somehow
cleaning up the existing code.
For now, I've stashed these changes off to the side. I could spin a v4
of the vhost-scsi series without any of the s/host/scsihost/ variations
you asked for, but this rabbit hole is probably going to consume me
until the next freeze/holiday. Thoughts?
I've been heads down in some vHBA/NPIV code - less than friendly
stuff... It's a customer case, so it's taken priority...
Quick thoughts -
... the RFC used SCSISCSI which really looked odd and I think I
originally avoided for that very reason when I gone through a similar
exercise some time ago.
... since the first issues that caught my attention were in patch4,
maybe attack those first - that is change 'Host' to 'SCSCIVHost'
API's
... for this patch, it may just be best to change HOST to VHOST and Host
to VHost.
I'll try to put some more thought into it in the morning...
John
[...]