src/conf/domain_conf.c | 11 +---------- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 6 ++++++ tests/qemuhotplugtest.c | 2 +- .../qemuhotplug-disk-scsi-2.xml | 2 +- ...-base-without-scsi-controller-live+disk-scsi-2.xml | 4 ++-- 7 files changed, 16 insertions(+), 14 deletions(-)
This is an alternative approach to: https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html which caused regression to which a proposed fix is here: https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html One of the selling points was that having this check in virDomainDefCompatibleDevice() will fix the problem across all drivers. Well, turns out, some drivers don't care (e.g. lxc) and some have a different workflow and thus don't need that check. Michal Prívozník (4): conf: Expose virDomainSCSIDriveAddressIsUsed qemuhotplugtest: Don't plug a SCSI disk at unit 7 qemu_hotplug: Check for duplicate drive addresses Revert "domain_conf: check device address before attach" src/conf/domain_conf.c | 11 +---------- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 6 ++++++ tests/qemuhotplugtest.c | 2 +- .../qemuhotplug-disk-scsi-2.xml | 2 +- ...-base-without-scsi-controller-live+disk-scsi-2.xml | 4 ++-- 7 files changed, 16 insertions(+), 14 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/11/19 11:34 AM, Michal Privoznik wrote: > This is an alternative approach to: > > https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html > > which caused regression to which a proposed fix is here: > > https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html > > One of the selling points was that having this check in > virDomainDefCompatibleDevice() will fix the problem across all drivers. > Well, turns out, some drivers don't care (e.g. lxc) and some have a > different workflow and thus don't need that check. Just tested the patch series and it fixes the problem for SCSI disks, as expected. The major drawback of reverting that patch is that we lose the address check not just for other drivers (that might not care much for that), but also for the remaining device types other than SCSI disks. Now, taking a quick look in the code I see that there are similar checks being made in VDA, PCI and DIMM type devices (didn't check the remaining devices). If there are address redundancy checks for all other devices, then the patch can be reverted without loss. It is worth mentioning that the first 2 patches can be applied regardless of what it is decided to do about the address checking, since they are a straight improvement in what already exists. Thanks, DHB > > Michal Prívozník (4): > conf: Expose virDomainSCSIDriveAddressIsUsed > qemuhotplugtest: Don't plug a SCSI disk at unit 7 > qemu_hotplug: Check for duplicate drive addresses > Revert "domain_conf: check device address before attach" > > src/conf/domain_conf.c | 11 +---------- > src/conf/domain_conf.h | 4 ++++ > src/libvirt_private.syms | 1 + > src/qemu/qemu_hotplug.c | 6 ++++++ > tests/qemuhotplugtest.c | 2 +- > .../qemuhotplug-disk-scsi-2.xml | 2 +- > ...-base-without-scsi-controller-live+disk-scsi-2.xml | 4 ++-- > 7 files changed, 16 insertions(+), 14 deletions(-) > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/11/19 8:36 PM, Daniel Henrique Barboza wrote: > > > On 4/11/19 11:34 AM, Michal Privoznik wrote: >> This is an alternative approach to: >> >> https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html >> >> which caused regression to which a proposed fix is here: >> >> https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html >> >> One of the selling points was that having this check in >> virDomainDefCompatibleDevice() will fix the problem across all drivers. >> Well, turns out, some drivers don't care (e.g. lxc) and some have a >> different workflow and thus don't need that check. > > Just tested the patch series and it fixes the problem for SCSI disks, as > expected. > > The major drawback of reverting that patch is that we lose the address > check not just for other drivers (that might not care much for that), but > also for the remaining device types other than SCSI disks. > > Now, taking a quick look in the code I see that there are similar checks > being made in VDA, PCI and DIMM type devices (didn't check the remaining > devices). If there are address redundancy checks for all other devices, > then > the patch can be reverted without loss. Yep, other types of addresees are covered. At least in drivers that do care. LXC doesn't because it doesn't make much sense there. I mean, a container shares HW with the host, so we can't really make it appear at different addresses. For instance, when plugging a disk into an LXC container it'll appear at the same adddess as in the host and no <address/> in the XML is going to change that. That's why I vouche for this approach. Sorry for not realizing this the first time. > > It is worth mentioning that the first 2 patches can be applied > regardless of > what it is decided to do about the address checking, since they are a > straight > improvement in what already exists. Thanks, Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/12/19 3:16 AM, Michal Privoznik wrote: > On 4/11/19 8:36 PM, Daniel Henrique Barboza wrote: >> >> >> On 4/11/19 11:34 AM, Michal Privoznik wrote: >>> This is an alternative approach to: >>> >>> https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html >>> >>> which caused regression to which a proposed fix is here: >>> >>> https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html >>> >>> One of the selling points was that having this check in >>> virDomainDefCompatibleDevice() will fix the problem across all drivers. >>> Well, turns out, some drivers don't care (e.g. lxc) and some have a >>> different workflow and thus don't need that check. >> >> Just tested the patch series and it fixes the problem for SCSI disks, as >> expected. >> >> The major drawback of reverting that patch is that we lose the address >> check not just for other drivers (that might not care much for that), but >> also for the remaining device types other than SCSI disks. >> >> Now, taking a quick look in the code I see that there are similar checks >> being made in VDA, PCI and DIMM type devices (didn't check the remaining >> devices). If there are address redundancy checks for all other devices, then >> the patch can be reverted without loss. > > Yep, other types of addresees are covered. At least in drivers that do care. LXC > doesn't because it doesn't make much sense there. I mean, a container shares HW > with the host, so we can't really make it appear at different addresses. For > instance, when plugging a disk into an LXC container it'll appear at the same > adddess as in the host and no <address/> in the XML is going to change that. > > That's why I vouche for this approach. Sorry for not realizing this the first time. I agree with this approach as well. IMO the check should be done in the hypervisor drivers, where there is more knowledge about what is permissible wrt the various device types. Regards, Jim > >> >> It is worth mentioning that the first 2 patches can be applied regardless of >> what it is decided to do about the address checking, since they are a straight >> improvement in what already exists. > > Thanks, > > Michal > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/12/19 6:16 AM, Michal Privoznik wrote: > On 4/11/19 8:36 PM, Daniel Henrique Barboza wrote: >> >> >> On 4/11/19 11:34 AM, Michal Privoznik wrote: >>> This is an alternative approach to: >>> >>> https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html >>> >>> which caused regression to which a proposed fix is here: >>> >>> https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html >>> >>> One of the selling points was that having this check in >>> virDomainDefCompatibleDevice() will fix the problem across all drivers. >>> Well, turns out, some drivers don't care (e.g. lxc) and some have a >>> different workflow and thus don't need that check. >> >> Just tested the patch series and it fixes the problem for SCSI disks, as >> expected. >> >> The major drawback of reverting that patch is that we lose the address >> check not just for other drivers (that might not care much for that), >> but >> also for the remaining device types other than SCSI disks. >> >> Now, taking a quick look in the code I see that there are similar checks >> being made in VDA, PCI and DIMM type devices (didn't check the remaining >> devices). If there are address redundancy checks for all other >> devices, then >> the patch can be reverted without loss. > > Yep, other types of addresees are covered. At least in drivers that do > care. LXC doesn't because it doesn't make much sense there. I mean, a > container shares HW with the host, so we can't really make it appear > at different addresses. For instance, when plugging a disk into an LXC > container it'll appear at the same adddess as in the host and no > <address/> in the XML is going to change that. > > That's why I vouche for this approach. Sorry for not realizing this > the first time. That makes sense. For all patches: Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > >> >> It is worth mentioning that the first 2 patches can be applied >> regardless of >> what it is decided to do about the address checking, since they are a >> straight >> improvement in what already exists. > > Thanks, > > Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/11/19 8:34 AM, Michal Privoznik wrote: > This is an alternative approach to: > > https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html > > which caused regression to which a proposed fix is here: > > https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html > > One of the selling points was that having this check in > virDomainDefCompatibleDevice() will fix the problem across all drivers. > Well, turns out, some drivers don't care (e.g. lxc) and some have a > different workflow and thus don't need that check. > > Michal Prívozník (4): > conf: Expose virDomainSCSIDriveAddressIsUsed > qemuhotplugtest: Don't plug a SCSI disk at unit 7 > qemu_hotplug: Check for duplicate drive addresses > Revert "domain_conf: check device address before attach" > > src/conf/domain_conf.c | 11 +---------- > src/conf/domain_conf.h | 4 ++++ > src/libvirt_private.syms | 1 + > src/qemu/qemu_hotplug.c | 6 ++++++ > tests/qemuhotplugtest.c | 2 +- > .../qemuhotplug-disk-scsi-2.xml | 2 +- > ...-base-without-scsi-controller-live+disk-scsi-2.xml | 4 ++-- > 7 files changed, 16 insertions(+), 14 deletions(-) > Thanks. I reviewed and tested the series. Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/12/19 7:25 PM, Jim Fehlig wrote: > On 4/11/19 8:34 AM, Michal Privoznik wrote: >> This is an alternative approach to: >> >> https://www.redhat.com/archives/libvir-list/2019-March/msg01111.html >> >> which caused regression to which a proposed fix is here: >> >> https://www.redhat.com/archives/libvir-list/2019-April/msg00756.html >> >> One of the selling points was that having this check in >> virDomainDefCompatibleDevice() will fix the problem across all drivers. >> Well, turns out, some drivers don't care (e.g. lxc) and some have a >> different workflow and thus don't need that check. >> >> Michal Prívozník (4): >> conf: Expose virDomainSCSIDriveAddressIsUsed >> qemuhotplugtest: Don't plug a SCSI disk at unit 7 >> qemu_hotplug: Check for duplicate drive addresses >> Revert "domain_conf: check device address before attach" >> >> src/conf/domain_conf.c | 11 +---------- >> src/conf/domain_conf.h | 4 ++++ >> src/libvirt_private.syms | 1 + >> src/qemu/qemu_hotplug.c | 6 ++++++ >> tests/qemuhotplugtest.c | 2 +- >> .../qemuhotplug-disk-scsi-2.xml | 2 +- >> ...-base-without-scsi-controller-live+disk-scsi-2.xml | 4 ++-- >> 7 files changed, 16 insertions(+), 14 deletions(-) >> > > Thanks. I reviewed and tested the series. > > Reviewed-by: Jim Fehlig <jfehlig@suse.com> Oh forgot to push those. Sorry. Now pushed. Thank you both. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.