Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/11/19 5:12 AM, Peter Krempa wrote: > On Thu, Oct 10, 2019 at 14:08:12 -0400, John Snow wrote: >> On 10/10/19 7:54 AM, Peter Krempa wrote: >>> On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote: On 10/10/19 1:26 PM, Peter Krempa wrote: > > [...] > >>> To be honest, I didn't really review the code nor the documentation. >>> I actually reviewed only the idea itself in the context of integration >>> with libvirt and that's why I didn't go for 'Reviewed-by:'. >>> >>> The gist of the citation above is that we should stick to well known >>> tags with their well known meanings and I think that considering this a >>> 'review' would be a stretch of the definiton. >>> >> >> I wasn't aware that PMM wanted to avoid non-standard tags; I consider >> them to be for human use, but I can change that behavior. >> >> Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you. > > Sure! I'll spell it out even for compliance: > > ACKed-by: Peter Krempa > Thanks, applied to my IDE tree: https://github.com/jnsnow/qemu/commits/ide https://github.com/jnsnow/qemu.git --js -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On Thu, Oct 10, 2019 at 14:08:12 -0400, John Snow wrote: > On 10/10/19 7:54 AM, Peter Krempa wrote: > > On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote: > >> On 10/10/19 1:26 PM, Peter Krempa wrote: [...] > > To be honest, I didn't really review the code nor the documentation. > > I actually reviewed only the idea itself in the context of integration > > with libvirt and that's why I didn't go for 'Reviewed-by:'. > > > > The gist of the citation above is that we should stick to well known > > tags with their well known meanings and I think that considering this a > > 'review' would be a stretch of the definiton. > > > > I wasn't aware that PMM wanted to avoid non-standard tags; I consider > them to be for human use, but I can change that behavior. > > Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you. Sure! I'll spell it out even for compliance: ACKed-by: Peter Krempa signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/10/19 7:54 AM, Peter Krempa wrote: > On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote: >> On 10/10/19 1:26 PM, Peter Krempa wrote: >>> On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: On 10/10/19 12:43 AM, John Snow wrote: > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > I'd like to refactor these some day, and getting rid of the super-object > will make that easier. > > Either way, we don't need this. > > Libvirt-checked-by: Peter Krempa Peter made a comment regarding Laszlo's Regression-tested-by tag: [...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction... It probably applies to 'Libvirt-checked-by' too. >>> >>> I certainly didn't test it. So feel free to drop that line altogether. >> >> But you reviewed it, can we use your 'Reviewed-by' instead? > > To be honest, I didn't really review the code nor the documentation. > I actually reviewed only the idea itself in the context of integration > with libvirt and that's why I didn't go for 'Reviewed-by:'. > > The gist of the citation above is that we should stick to well known > tags with their well known meanings and I think that considering this a > 'review' would be a stretch of the definiton. > I wasn't aware that PMM wanted to avoid non-standard tags; I consider them to be for human use, but I can change that behavior. Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you. --js -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote: > On 10/10/19 1:26 PM, Peter Krempa wrote: > > On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: > > > On 10/10/19 12:43 AM, John Snow wrote: > > > > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > > > > I'd like to refactor these some day, and getting rid of the super-object > > > > will make that easier. > > > > > > > > Either way, we don't need this. > > > > > > > > Libvirt-checked-by: Peter Krempa > > > > > > Peter made a comment regarding Laszlo's Regression-tested-by tag: > > > > > >[...] nobody else is using > > >this convention (there are exactly 0 instances of > > >"Regression-tested-by" in the project git log as far as > > >I can see), and so in practice people reading the commits > > >won't really know what you meant by it. Everybody else > > >on the project uses "Tested-by" to mean either of the > > >two cases you describe above, without distinction... > > > > > > It probably applies to 'Libvirt-checked-by' too. > > > > I certainly didn't test it. So feel free to drop that line altogether. > > But you reviewed it, can we use your 'Reviewed-by' instead? To be honest, I didn't really review the code nor the documentation. I actually reviewed only the idea itself in the context of integration with libvirt and that's why I didn't go for 'Reviewed-by:'. The gist of the citation above is that we should stick to well known tags with their well known meanings and I think that considering this a 'review' would be a stretch of the definiton. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/10/19 1:26 PM, Peter Krempa wrote: On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: On 10/10/19 12:43 AM, John Snow wrote: It's an old compatibility shim that just delegates to ide-cd or ide-hd. I'd like to refactor these some day, and getting rid of the super-object will make that easier. Either way, we don't need this. Libvirt-checked-by: Peter Krempa Peter made a comment regarding Laszlo's Regression-tested-by tag: [...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction... It probably applies to 'Libvirt-checked-by' too. I certainly didn't test it. So feel free to drop that line altogether. But you reviewed it, can we use your 'Reviewed-by' instead? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: > On 10/10/19 12:43 AM, John Snow wrote: > > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > > I'd like to refactor these some day, and getting rid of the super-object > > will make that easier. > > > > Either way, we don't need this. > > > > Libvirt-checked-by: Peter Krempa > > Peter made a comment regarding Laszlo's Regression-tested-by tag: > > [...] nobody else is using > this convention (there are exactly 0 instances of > "Regression-tested-by" in the project git log as far as > I can see), and so in practice people reading the commits > won't really know what you meant by it. Everybody else > on the project uses "Tested-by" to mean either of the > two cases you describe above, without distinction... > > It probably applies to 'Libvirt-checked-by' too. I certainly didn't test it. So feel free to drop that line altogether. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/10/2019 00.43, John Snow wrote: > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > I'd like to refactor these some day, and getting rid of the super-object > will make that easier. > > Either way, we don't need this. > > Libvirt-checked-by: Peter Krempa > Signed-off-by: John Snow > --- > qemu-deprecated.texi | 5 + > hw/ide/qdev.c | 3 +++ > tests/qemu-iotests/051.pc.out | 6 -- > 3 files changed, 12 insertions(+), 2 deletions(-) Reviewed-by: Thomas Huth -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
John Snow writes: > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > I'd like to refactor these some day, and getting rid of the super-object > will make that easier. > > Either way, we don't need this. > > Libvirt-checked-by: Peter Krempa > Signed-off-by: John Snow Reviewed-by: Markus Armbruster -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
It's an old compatibility shim that just delegates to ide-cd or ide-hd.
I'd like to refactor these some day, and getting rid of the super-object
will make that easier.
Either way, we don't need this.
Libvirt-checked-by: Peter Krempa
Signed-off-by: John Snow
---
qemu-deprecated.texi | 5 +
hw/ide/qdev.c | 3 +++
tests/qemu-iotests/051.pc.out | 6 --
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 01245e0b1c4..7cd4648df3c 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -247,6 +247,11 @@ quite a bit. It will be removed without replacement unless
some users speaks
up at the @email{qemu-devel@@nongnu.org} mailing list with information about
their usecases.
+@subsection ide-drive (since 4.2)
+
+The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
+'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
+
@section System emulator machines
@subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6fba6b62b87..3666e597211 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -279,6 +279,9 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
{
DriveInfo *dinfo = NULL;
+warn_report("'ide-drive' is deprecated, "
+"please use 'ide-hd' or 'ide-cd' instead");
+
if (dev->conf.blk) {
dinfo = blk_legacy_dinfo(dev->conf.blk);
}
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 000557c7c83..34849dd1720 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -158,7 +158,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
Testing: -drive if=none,id=disk -device ide-drive,drive=disk
QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive
is empty
+(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: 'ide-drive' is
deprecated, please use 'ide-hd' or 'ide-cd' instead
+QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
Testing: -drive if=none,id=disk -device ide-hd,drive=disk
QEMU X.Y.Z monitor - type 'help' for more information
@@ -228,7 +229,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device
ide-drive,drive=disk
QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
+(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: 'ide-drive' is
deprecated, please use 'ide-hd' or 'ide-cd' instead
+QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device
ide-hd,drive=disk
QEMU X.Y.Z monitor - type 'help' for more information
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
