Re: [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-03-07 Thread John Snow


On 03/07/2018 05:04 AM, Kevin Wolf wrote:
> Am 06.03.2018 um 22:51 hat John Snow geschrieben:
>> On 02/05/2018 03:22 PM, Max Reitz wrote:
>>> This series implements .bdrv_refresh_filename() for the ssh block
>>> driver, along with an appropriate .bdrv_dirname() so we don't chop off
>>> query strings for backing files with relative filenames.
>>>
>>> This series depends on my “block: Fix some filename generation issues”
>>> series and on Pino's “ssh: switch from libssh2 to libssh” patch.
>>>
>>> Based-on: 20180205151835.20812-1-mre...@redhat.com
>>> Based-on: 20180118164439.2120-1-ptosc...@redhat.com
>>>
>>>
>>> Max Reitz (2):
>>>   block/ssh: Implement .bdrv_refresh_filename()
>>>   block/ssh: Implement .bdrv_dirname()
>>>
>>>  block/ssh.c | 72 
>>> +++--
>>>  1 file changed, 65 insertions(+), 7 deletions(-)
>>
>> Did this one rot on the vine?
>>
>>> 1 month old.
> 
> The Based-on tags are the problem, in particular the first one. But yes,
> we could possibly do more to review the dependencies...
> 
> Kevin
> 

Yeah, sorry, I'm not pulling my review weight right now. I do try to
keep my block-devel inbox a rough "todo," though, so for patches that
get to that one month mark with no conclusion I like to ping them before
I archive them.

It's something I'd like to see patchew do, actually:

"Here's a list of what's on the list that has no reviews or NACKs, and
needs some love"

coupled with a 30 day "Hey, nobody looked at this" ping to the list
before it NACKs a set for being too old.

I hope nobody reads these >1 month pings as me wondering why nobody ELSE
has reviewed it, which is not my intent...

--js



Re: [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-03-07 Thread Kevin Wolf
Am 06.03.2018 um 22:51 hat John Snow geschrieben:
> On 02/05/2018 03:22 PM, Max Reitz wrote:
> > This series implements .bdrv_refresh_filename() for the ssh block
> > driver, along with an appropriate .bdrv_dirname() so we don't chop off
> > query strings for backing files with relative filenames.
> > 
> > This series depends on my “block: Fix some filename generation issues”
> > series and on Pino's “ssh: switch from libssh2 to libssh” patch.
> > 
> > Based-on: 20180205151835.20812-1-mre...@redhat.com
> > Based-on: 20180118164439.2120-1-ptosc...@redhat.com
> > 
> > 
> > Max Reitz (2):
> >   block/ssh: Implement .bdrv_refresh_filename()
> >   block/ssh: Implement .bdrv_dirname()
> > 
> >  block/ssh.c | 72 
> > +++--
> >  1 file changed, 65 insertions(+), 7 deletions(-)
> 
> Did this one rot on the vine?
> 
> >1 month old.

The Based-on tags are the problem, in particular the first one. But yes,
we could possibly do more to review the dependencies...

Kevin



Re: [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-03-06 Thread John Snow


On 02/05/2018 03:22 PM, Max Reitz wrote:
> This series implements .bdrv_refresh_filename() for the ssh block
> driver, along with an appropriate .bdrv_dirname() so we don't chop off
> query strings for backing files with relative filenames.
> 
> This series depends on my “block: Fix some filename generation issues”
> series and on Pino's “ssh: switch from libssh2 to libssh” patch.
> 
> Based-on: 20180205151835.20812-1-mre...@redhat.com
> Based-on: 20180118164439.2120-1-ptosc...@redhat.com
> 
> 
> Max Reitz (2):
>   block/ssh: Implement .bdrv_refresh_filename()
>   block/ssh: Implement .bdrv_dirname()
> 
>  block/ssh.c | 72 
> +++--
>  1 file changed, 65 insertions(+), 7 deletions(-)
> 

Did this one rot on the vine?

>1 month old.



Re: [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-02-05 Thread Max Reitz
On 2018-02-05 21:46, Richard W.M. Jones wrote:
> 
> I assume the context of all this is making relative backing file names work
> in qcow2 files?

Yes, mostly.  (https://bugzilla.redhat.com/show_bug.cgi?id=1528931)
But it's generally nice for block nodes to print a readable filename
when queried and not a json:{} monster all of the time.

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-02-05 Thread Richard W.M. Jones

I assume the context of all this is making relative backing file names work
in qcow2 files?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html



Re: [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-02-05 Thread Richard W.M. Jones
On Mon, Feb 05, 2018 at 09:22:30PM +0100, Max Reitz wrote:
> This series implements .bdrv_refresh_filename() for the ssh block
> driver, along with an appropriate .bdrv_dirname() so we don't chop off
> query strings for backing files with relative filenames.
> 
> This series depends on my “block: Fix some filename generation issues”
> series and on Pino's “ssh: switch from libssh2 to libssh” patch.
> 
> Based-on: 20180205151835.20812-1-mre...@redhat.com
> Based-on: 20180118164439.2120-1-ptosc...@redhat.com
> 
> 
> Max Reitz (2):
>   block/ssh: Implement .bdrv_refresh_filename()
>   block/ssh: Implement .bdrv_dirname()
> 
>  block/ssh.c | 72 
> +++--

I reviewed the patches and I can't see anything wrong with them.

I guess that on "weird" remote machines getting the directory from a
filename is an impossible mission, but for reasonable hosts it's going
to be fine.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org