[PATCH 0/3] Rmove support of tftp storage protocol

Han Han posted 3 patches 3 years, 10 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200521060820.156471-1-hhan@redhat.com
docs/formatdomain.html.in     | 10 +---------
docs/news.xml                 | 11 +++++++++++
docs/schemas/domaincommon.rng |  1 -
src/libxl/libxl_conf.c        |  1 -
src/libxl/xen_xl.c            |  1 -
src/qemu/qemu_block.c         |  3 ---
src/qemu/qemu_command.c       |  1 -
src/qemu/qemu_domain.c        | 11 -----------
src/qemu/qemu_driver.c        |  3 ---
src/util/virstoragefile.c     |  6 ------
src/util/virstoragefile.h     |  1 -
11 files changed, 12 insertions(+), 37 deletions(-)
[PATCH 0/3] Rmove support of tftp storage protocol
Posted by Han Han 3 years, 10 months ago
Let's drop tftp support since it is ususabled before or after QEMU v2.8
[1].

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1372143


Han Han (3):
  docs: Remove tftp protocol support from docs
  src: Remove tftp network storage support
  news: Remove support for tftp storage protocol

 docs/formatdomain.html.in     | 10 +---------
 docs/news.xml                 | 11 +++++++++++
 docs/schemas/domaincommon.rng |  1 -
 src/libxl/libxl_conf.c        |  1 -
 src/libxl/xen_xl.c            |  1 -
 src/qemu/qemu_block.c         |  3 ---
 src/qemu/qemu_command.c       |  1 -
 src/qemu/qemu_domain.c        | 11 -----------
 src/qemu/qemu_driver.c        |  3 ---
 src/util/virstoragefile.c     |  6 ------
 src/util/virstoragefile.h     |  1 -
 11 files changed, 12 insertions(+), 37 deletions(-)

-- 
2.25.0

Re: [PATCH 0/3] Rmove support of tftp storage protocol
Posted by Peter Krempa 3 years, 10 months ago
On Thu, May 21, 2020 at 14:08:17 +0800, Han Han wrote:
> Let's drop tftp support since it is ususabled before or after QEMU v2.8
> [1].
> 
> [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1372143
> 
> 
> Han Han (3):
>   docs: Remove tftp protocol support from docs
>   src: Remove tftp network storage support
>   news: Remove support for tftp storage protocol

NACK series:

1) qemu 2.8 is still supported by libvirt
2) we can't remove XML parser support for previously supported XMLs as
the existing defined VMs would vanish. That's exactly why the check in
my patch was added into the validator which doesn't validate existing
defined configs.

Re: [PATCH 0/3] Rmove support of tftp storage protocol
Posted by Han Han 3 years, 10 months ago
On Thu, May 21, 2020 at 2:44 PM Peter Krempa <pkrempa@redhat.com> wrote:

> On Thu, May 21, 2020 at 14:08:17 +0800, Han Han wrote:
> > Let's drop tftp support since it is ususabled before or after QEMU v2.8
> > [1].
> >
> > [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1372143
> >
> >
> > Han Han (3):
> >   docs: Remove tftp protocol support from docs
> >   src: Remove tftp network storage support
> >   news: Remove support for tftp storage protocol
>
> NACK series:
>
> 1) qemu 2.8 is still supported by libvirt
>
However, the tftp is actually unusable for all versions of qemu, while the
formatdomain doc
states that it is optional protocol

> 2) we can't remove XML parser support for previously supported XMLs as
> the existing defined VMs would vanish. That's exactly why the check in
> my patch was added into the validator which doesn't validate existing
> defined configs.
>
I understand the consideration of the compatibility. How about only
removing it from docs?


-- 
Best regards,
-----------------------------------
Han Han
Quality Engineer
Redhat.

Email: hhan@redhat.com
Phone: +861065339333
Re: [PATCH 0/3] Rmove support of tftp storage protocol
Posted by Daniel P. Berrangé 3 years, 10 months ago
On Thu, May 21, 2020 at 04:05:47PM +0800, Han Han wrote:
> On Thu, May 21, 2020 at 2:44 PM Peter Krempa <pkrempa@redhat.com> wrote:
> 
> > On Thu, May 21, 2020 at 14:08:17 +0800, Han Han wrote:
> > > Let's drop tftp support since it is ususabled before or after QEMU v2.8
> > > [1].
> > >
> > > [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1372143
> > >
> > >
> > > Han Han (3):
> > >   docs: Remove tftp protocol support from docs
> > >   src: Remove tftp network storage support
> > >   news: Remove support for tftp storage protocol
> >
> > NACK series:
> >
> > 1) qemu 2.8 is still supported by libvirt
> >
> However, the tftp is actually unusable for all versions of qemu, while the
> formatdomain doc
> states that it is optional protocol

Do you have any more information on this. I'd be surprised if it was
/always/ unusuable, as opposed to initially working and then getting
broken in some particular version of QEMU.

> 
> > 2) we can't remove XML parser support for previously supported XMLs as
> > the existing defined VMs would vanish. That's exactly why the check in
> > my patch was added into the validator which doesn't validate existing
> > defined configs.
> >
> I understand the consideration of the compatibility. How about only
> removing it from docs?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 0/3] Rmove support of tftp storage protocol
Posted by Han Han 3 years, 10 months ago
On Thu, May 21, 2020 at 5:21 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, May 21, 2020 at 04:05:47PM +0800, Han Han wrote:
> > On Thu, May 21, 2020 at 2:44 PM Peter Krempa <pkrempa@redhat.com> wrote:
> >
> > > On Thu, May 21, 2020 at 14:08:17 +0800, Han Han wrote:
> > > > Let's drop tftp support since it is ususabled before or after QEMU
> v2.8
> > > > [1].
> > > >
> > > > [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1372143
> > > >
> > > >
> > > > Han Han (3):
> > > >   docs: Remove tftp protocol support from docs
> > > >   src: Remove tftp network storage support
> > > >   news: Remove support for tftp storage protocol
> > >
> > > NACK series:
> > >
> > > 1) qemu 2.8 is still supported by libvirt
> > >
> > However, the tftp is actually unusable for all versions of qemu, while
> the
> > formatdomain doc
> > states that it is optional protocol
>
> Do you have any more information on this. I'd be surprised if it was
> /always/ unusuable, as opposed to initially working and then getting
> broken in some particular version of QEMU.
>
I tested on qemu-kvm-1.5.3-174.el7.x86_64 curl-7.29.0-58.el7.x86_64
tftp-server-5.2-22.el7.x86_64, it doesn't work, either:
➜  ~ curl tftp://127.0.0.1/img -o /tmp/tftp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time
 Current
                                 Dload  Upload   Total   Spent    Left
 Speed
100  100M  100  100M    0     0  20.6M      0  0:00:04  0:00:04 --:--:--
20.6M
100  100M  100  100M    0     0  20.6M      0  0:00:04  0:00:04 --:--:--
20.6M
➜  ~ qemu-img create -f qcow2 -b 'json:{"file.driver":"tftp",
"file.url":"tftp://127.0.0.1/img"}' /var/lib/libvirt/images/tftp.img
Formatting '/var/lib/libvirt/images/tftp.img', fmt=qcow2 size=104857600
backing_file='json:{"file.driver":"tftp", "file.url":"tftp://127.0.0.1/img"}'
encryption=off cluster_size=65536 lazy_refcounts=off
^C

➜  ~ qemu-img info 'json:{"file.driver":"tftp", "file.url":"tftp://
127.0.0.1/img"}'
^C

The qemu-img command above is stuck and never returns.

Max, is the tftp unusable for all versions of QEMU? Could you please give
more info on that?

>
> >
> > > 2) we can't remove XML parser support for previously supported XMLs as
> > > the existing defined VMs would vanish. That's exactly why the check in
> > > my patch was added into the validator which doesn't validate existing
> > > defined configs.
> > >
> > I understand the consideration of the compatibility. How about only
> > removing it from docs?
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>

-- 
Best regards,
-----------------------------------
Han Han
Quality Engineer
Redhat.

Email: hhan@redhat.com
Phone: +861065339333
Re: [PATCH 0/3] Rmove support of tftp storage protocol
Posted by Max Reitz 3 years, 10 months ago
On 22.05.20 04:07, Han Han wrote:
> 
> 
> On Thu, May 21, 2020 at 5:21 PM Daniel P. Berrangé <berrange@redhat.com
> <mailto:berrange@redhat.com>> wrote:
> 
>     On Thu, May 21, 2020 at 04:05:47PM +0800, Han Han wrote:
>     > On Thu, May 21, 2020 at 2:44 PM Peter Krempa <pkrempa@redhat.com
>     <mailto:pkrempa@redhat.com>> wrote:
>     >
>     > > On Thu, May 21, 2020 at 14:08:17 +0800, Han Han wrote:
>     > > > Let's drop tftp support since it is ususabled before or after
>     QEMU v2.8
>     > > > [1].
>     > > >
>     > > > [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1372143
>     > > >
>     > > >
>     > > > Han Han (3):
>     > > >   docs: Remove tftp protocol support from docs
>     > > >   src: Remove tftp network storage support
>     > > >   news: Remove support for tftp storage protocol
>     > >
>     > > NACK series:
>     > >
>     > > 1) qemu 2.8 is still supported by libvirt
>     > >
>     > However, the tftp is actually unusable for all versions of qemu,
>     while the
>     > formatdomain doc
>     > states that it is optional protocol
> 
>     Do you have any more information on this. I'd be surprised if it was
>     /always/ unusuable, as opposed to initially working and then getting
>     broken in some particular version of QEMU.
> 
> I tested on qemu-kvm-1.5.3-174.el7.x86_64 curl-7.29.0-58.el7.x86_64
> tftp-server-5.2-22.el7.x86_64, it doesn't work, either:
> ➜  ~ curl tftp://127.0.0.1/img <http://127.0.0.1/img> -o /tmp/tftp      
>                          
>   % Total    % Received % Xferd  Average Speed   Time    Time     Time
>  Current
>                                  Dload  Upload   Total   Spent    Left
>  Speed
> 100  100M  100  100M    0     0  20.6M      0  0:00:04  0:00:04 --:--:--
> 20.6M
> 100  100M  100  100M    0     0  20.6M      0  0:00:04  0:00:04 --:--:--
> 20.6M
> ➜  ~ qemu-img create -f qcow2 -b 'json:{"file.driver":"tftp",
> "file.url":"tftp://127.0.0.1/img <http://127.0.0.1/img>"}'
> /var/lib/libvirt/images/tftp.img
> Formatting '/var/lib/libvirt/images/tftp.img', fmt=qcow2 size=104857600
> backing_file='json:{"file.driver":"tftp",
> "file.url":"tftp://127.0.0.1/img <http://127.0.0.1/img>"}'
> encryption=off cluster_size=65536 lazy_refcounts=off
> ^C
> 
> ➜  ~ qemu-img info 'json:{"file.driver":"tftp",
> "file.url":"tftp://127.0.0.1/img <http://127.0.0.1/img>"}'
> ^C
> 
> The qemu-img command above is stuck and never returns.
> 
> Max, is the tftp unusable for all versions of QEMU? Could you please
> give more info on that?

I have no idea.  I’ve never tested it.

Max