xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
@ 2016-03-17 22:10 Ross Philipson
  2016-03-25 16:11 ` Wei Liu
  2016-04-06 14:39 ` Ian Jackson
  0 siblings, 2 replies; 12+ messages in thread
From: Ross Philipson @ 2016-03-17 22:10 UTC (permalink / raw)
  To: xen-devel

It seems the logic is meant to detect sector unaligned buffers for block
writes. The NOTing of the logic instead masks off any unaligned bits and
also would cause the function to always fail. It seems the function is not
used in any of the tools so that is probably why the problem is not seen.
In the vhd_read_block function it is correct.

Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
---
diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
index 1fd5b4e..4ebe012 100644
--- a/tools/blktap2/vhd/lib/libvhd.c
+++ b/tools/blktap2/vhd/lib/libvhd.c
@@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char *data)
        if (block >= ctx->bat.entries)
                return -ERANGE;

-       if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
+       if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
                return -EINVAL;

        blk  = ctx->bat.bat[block];

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-17 22:10 [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block Ross Philipson
@ 2016-03-25 16:11 ` Wei Liu
  2016-03-25 16:34   ` Ross Philipson
  2016-04-06 14:39 ` Ian Jackson
  1 sibling, 1 reply; 12+ messages in thread
From: Wei Liu @ 2016-03-25 16:11 UTC (permalink / raw)
  To: Ross Philipson; +Cc: Wei Liu, xen-devel

On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
> It seems the logic is meant to detect sector unaligned buffers for block
> writes. The NOTing of the logic instead masks off any unaligned bits and
> also would cause the function to always fail. It seems the function is not
> used in any of the tools so that is probably why the problem is not seen.
> In the vhd_read_block function it is correct.
> 
> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>

This seems to fall under tools umbrella. I've look at blktap2 code,
the reasoning is convincing to me so:

  Acked-by: Wei Liu <wei.liu2@citrix.com>

But I've never used blktap2 and we don't normally test it so I would
also wait a bit longer to see if anybody objects to this change.

OOI if no code was using this, how did you discover this problem?

Wei.

> ---
> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
> index 1fd5b4e..4ebe012 100644
> --- a/tools/blktap2/vhd/lib/libvhd.c
> +++ b/tools/blktap2/vhd/lib/libvhd.c
> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char *data)
>         if (block >= ctx->bat.entries)
>                 return -ERANGE;
> 
> -       if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
> +       if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
>                 return -EINVAL;
> 
>         blk  = ctx->bat.bat[block];
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-25 16:11 ` Wei Liu
@ 2016-03-25 16:34   ` Ross Philipson
  2016-03-25 16:38     ` Wei Liu
  2016-03-29 11:26     ` George Dunlap
  0 siblings, 2 replies; 12+ messages in thread
From: Ross Philipson @ 2016-03-25 16:34 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel

On 03/25/2016 12:11 PM, Wei Liu wrote:
> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>> It seems the logic is meant to detect sector unaligned buffers for block
>> writes. The NOTing of the logic instead masks off any unaligned bits and
>> also would cause the function to always fail. It seems the function is not
>> used in any of the tools so that is probably why the problem is not seen.
>> In the vhd_read_block function it is correct.
>>
>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
> 
> This seems to fall under tools umbrella. I've look at blktap2 code,
> the reasoning is convincing to me so:
> 
>   Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> But I've never used blktap2 and we don't normally test it so I would
> also wait a bit longer to see if anybody objects to this change.
> 
> OOI if no code was using this, how did you discover this problem?

We have an old fork of blktap2 from way back when. I was working to extract our
changes and turn them into patches so we could rebase on upstream blktap2.
Someone long ago found that - I have no idea how but it was a valid fix so I
upstreamed it.

There are a number of other ones that were found that are still in upstream
blktap2 - I plan to send them too.

> 
> Wei.
> 
>> ---
>> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
>> index 1fd5b4e..4ebe012 100644
>> --- a/tools/blktap2/vhd/lib/libvhd.c
>> +++ b/tools/blktap2/vhd/lib/libvhd.c
>> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char *data)
>>         if (block >= ctx->bat.entries)
>>                 return -ERANGE;
>>
>> -       if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
>> +       if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
>>                 return -EINVAL;
>>
>>         blk  = ctx->bat.bat[block];
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel


-- 
Ross Philipson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-25 16:34   ` Ross Philipson
@ 2016-03-25 16:38     ` Wei Liu
  2016-03-29 11:26     ` George Dunlap
  1 sibling, 0 replies; 12+ messages in thread
From: Wei Liu @ 2016-03-25 16:38 UTC (permalink / raw)
  To: Ross Philipson; +Cc: Wei Liu, xen-devel

On Fri, Mar 25, 2016 at 12:34:29PM -0400, Ross Philipson wrote:
> On 03/25/2016 12:11 PM, Wei Liu wrote:
> > On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
> >> It seems the logic is meant to detect sector unaligned buffers for block
> >> writes. The NOTing of the logic instead masks off any unaligned bits and
> >> also would cause the function to always fail. It seems the function is not
> >> used in any of the tools so that is probably why the problem is not seen.
> >> In the vhd_read_block function it is correct.
> >>
> >> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
> > 
> > This seems to fall under tools umbrella. I've look at blktap2 code,
> > the reasoning is convincing to me so:
> > 
> >   Acked-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > But I've never used blktap2 and we don't normally test it so I would
> > also wait a bit longer to see if anybody objects to this change.
> > 
> > OOI if no code was using this, how did you discover this problem?
> 
> We have an old fork of blktap2 from way back when. I was working to extract our
> changes and turn them into patches so we could rebase on upstream blktap2.
> Someone long ago found that - I have no idea how but it was a valid fix so I
> upstreamed it.
> 
> There are a number of other ones that were found that are still in upstream
> blktap2 - I plan to send them too.
> 

Alright, much appreciated!

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-25 16:34   ` Ross Philipson
  2016-03-25 16:38     ` Wei Liu
@ 2016-03-29 11:26     ` George Dunlap
  2016-03-30 14:47       ` Ross Philipson
  2016-04-06 16:51       ` Ross Philipson
  1 sibling, 2 replies; 12+ messages in thread
From: George Dunlap @ 2016-03-29 11:26 UTC (permalink / raw)
  To: Ross Philipson; +Cc: Wei Liu, xen-devel

On Fri, Mar 25, 2016 at 4:34 PM, Ross Philipson
<ross.philipson@gmail.com> wrote:
> On 03/25/2016 12:11 PM, Wei Liu wrote:
>> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>>> It seems the logic is meant to detect sector unaligned buffers for block
>>> writes. The NOTing of the logic instead masks off any unaligned bits and
>>> also would cause the function to always fail. It seems the function is not
>>> used in any of the tools so that is probably why the problem is not seen.
>>> In the vhd_read_block function it is correct.
>>>
>>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
>>
>> This seems to fall under tools umbrella. I've look at blktap2 code,
>> the reasoning is convincing to me so:
>>
>>   Acked-by: Wei Liu <wei.liu2@citrix.com>
>>
>> But I've never used blktap2 and we don't normally test it so I would
>> also wait a bit longer to see if anybody objects to this change.
>>
>> OOI if no code was using this, how did you discover this problem?
>
> We have an old fork of blktap2 from way back when. I was working to extract our
> changes and turn them into patches so we could rebase on upstream blktap2.
> Someone long ago found that - I have no idea how but it was a valid fix so I
> upstreamed it.
>
> There are a number of other ones that were found that are still in upstream
> blktap2 - I plan to send them too.

Ross,

Instead of cross-porting fixes from XenServer's blktap3 to the
bitrotting blktap2, would you consider taking up my work on replacing
blktap2 with building blktap3 as an external project?

The basic shape of things that need to happen for that are:

1. Allow qemu to provide emulated devices for disks which use hotplug
scripts (just checked in last week)

2. Do the tweaks necessary to allow blktap3 to be built as an
independent project (outside the XenServer build system)

3. Switch libxl to use the already-in-tree block-tap script (which
calls tapctl) rather than linking against the blktap library.

4. Add a blktap target to raisin.

#1 is done and was just checked in last week.  #2 shouldn't be a
terribly large amount of work.  For #3, I've posted patches already,
and I can probably do a quick rebase for you to pick up if you wanted.
#4 shouldn't be too hard once you have an out-of-tree build working; I
can help with that as well.

If we then add tests for upstream blktap to osstest, then we'll catch
any breakages, and automatically get updates; and we can delete the
bitrotting blktap2 out of the tree.

What do you think?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-29 11:26     ` George Dunlap
@ 2016-03-30 14:47       ` Ross Philipson
  2016-04-06 16:51       ` Ross Philipson
  1 sibling, 0 replies; 12+ messages in thread
From: Ross Philipson @ 2016-03-30 14:47 UTC (permalink / raw)
  To: George Dunlap; +Cc: Wei Liu, xen-devel

On 03/29/2016 07:26 AM, George Dunlap wrote:
> On Fri, Mar 25, 2016 at 4:34 PM, Ross Philipson
> <ross.philipson@gmail.com> wrote:
>> On 03/25/2016 12:11 PM, Wei Liu wrote:
>>> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>>>> It seems the logic is meant to detect sector unaligned buffers for block
>>>> writes. The NOTing of the logic instead masks off any unaligned bits and
>>>> also would cause the function to always fail. It seems the function is not
>>>> used in any of the tools so that is probably why the problem is not seen.
>>>> In the vhd_read_block function it is correct.
>>>>
>>>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
>>>
>>> This seems to fall under tools umbrella. I've look at blktap2 code,
>>> the reasoning is convincing to me so:
>>>
>>>   Acked-by: Wei Liu <wei.liu2@citrix.com>
>>>
>>> But I've never used blktap2 and we don't normally test it so I would
>>> also wait a bit longer to see if anybody objects to this change.
>>>
>>> OOI if no code was using this, how did you discover this problem?
>>
>> We have an old fork of blktap2 from way back when. I was working to extract our
>> changes and turn them into patches so we could rebase on upstream blktap2.
>> Someone long ago found that - I have no idea how but it was a valid fix so I
>> upstreamed it.
>>
>> There are a number of other ones that were found that are still in upstream
>> blktap2 - I plan to send them too.
> 
> Ross,
> 
> Instead of cross-porting fixes from XenServer's blktap3 to the
> bitrotting blktap2, would you consider taking up my work on replacing
> blktap2 with building blktap3 as an external project?

George, sorry I had to sort out some red tape before I could reply. For the
record I am not cross porting from blktap3 but upstreaming from a very old fork
of blktap2 that came from XenServer.

But really that is of little concern because I think your idea is a really good
one. Yes I would be up for taking on that work and we would very much like to
re-base our project on blktap3 too. I cannot give you an exact date when I would
start on it but probably I could start at the beginning of May. In the mean time
I will spend time familiarizing myself with blktap3 and I will probably have
some question for you too...

Thanks

> 
> The basic shape of things that need to happen for that are:
> 
> 1. Allow qemu to provide emulated devices for disks which use hotplug
> scripts (just checked in last week)
> 
> 2. Do the tweaks necessary to allow blktap3 to be built as an
> independent project (outside the XenServer build system)
> 
> 3. Switch libxl to use the already-in-tree block-tap script (which
> calls tapctl) rather than linking against the blktap library.
> 
> 4. Add a blktap target to raisin.
> 
> #1 is done and was just checked in last week.  #2 shouldn't be a
> terribly large amount of work.  For #3, I've posted patches already,
> and I can probably do a quick rebase for you to pick up if you wanted.
> #4 shouldn't be too hard once you have an out-of-tree build working; I
> can help with that as well.
> 
> If we then add tests for upstream blktap to osstest, then we'll catch
> any breakages, and automatically get updates; and we can delete the
> bitrotting blktap2 out of the tree.
> 
> What do you think?
> 
>  -George
> 


-- 
Ross Philipson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-17 22:10 [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block Ross Philipson
  2016-03-25 16:11 ` Wei Liu
@ 2016-04-06 14:39 ` Ian Jackson
  2016-04-06 15:25   ` Ross Philipson
  2016-04-06 15:37   ` Ross Philipson
  1 sibling, 2 replies; 12+ messages in thread
From: Ian Jackson @ 2016-04-06 14:39 UTC (permalink / raw)
  To: Ross Philipson; +Cc: xen-devel

Ross Philipson writes ("[Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block"):
> It seems the logic is meant to detect sector unaligned buffers for block
> writes. The NOTing of the logic instead masks off any unaligned bits and
> also would cause the function to always fail. It seems the function is not
> used in any of the tools so that is probably why the problem is not seen.
> In the vhd_read_block function it is correct.
> 
> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
> ---
> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
> index 1fd5b4e..4ebe012 100644
> --- a/tools/blktap2/vhd/lib/libvhd.c
> +++ b/tools/blktap2/vhd/lib/libvhd.c
> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char *data)
>         if (block >= ctx->bat.entries)
>                 return -ERANGE;
> 
> -       if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
> +       if ((unsigned long)data & (VHD_SECTOR_SIZE -1))

Notwithstanding the discussion in the rest of the thread, this is a
clearly-correct bugfix to the code in tree, so I have queued it.

However, I had to do so rather manually because something had mangled
the whitespace.  Ross, can you check your patch submission path,
please ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-04-06 14:39 ` Ian Jackson
@ 2016-04-06 15:25   ` Ross Philipson
  2016-04-06 15:37   ` Ross Philipson
  1 sibling, 0 replies; 12+ messages in thread
From: Ross Philipson @ 2016-04-06 15:25 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On 04/06/2016 10:39 AM, Ian Jackson wrote:
> Ross Philipson writes ("[Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block"):
>> It seems the logic is meant to detect sector unaligned buffers for block
>> writes. The NOTing of the logic instead masks off any unaligned bits and
>> also would cause the function to always fail. It seems the function is not
>> used in any of the tools so that is probably why the problem is not seen.
>> In the vhd_read_block function it is correct.
>>
>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
>> ---
>> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
>> index 1fd5b4e..4ebe012 100644
>> --- a/tools/blktap2/vhd/lib/libvhd.c
>> +++ b/tools/blktap2/vhd/lib/libvhd.c
>> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char *data)
>>         if (block >= ctx->bat.entries)
>>                 return -ERANGE;
>>
>> -       if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
>> +       if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
> 
> Notwithstanding the discussion in the rest of the thread, this is a
> clearly-correct bugfix to the code in tree, so I have queued it.
> 
> However, I had to do so rather manually because something had mangled
> the whitespace.  Ross, can you check your patch submission path,
> please ?
> 
> Ian.
> 

I went back and checked the original email I sent and I don't see any problems.
What part was mangled?

Thanks

-- 
Ross Philipson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-04-06 14:39 ` Ian Jackson
  2016-04-06 15:25   ` Ross Philipson
@ 2016-04-06 15:37   ` Ross Philipson
  2016-04-07 16:37     ` Ian Jackson
  1 sibling, 1 reply; 12+ messages in thread
From: Ross Philipson @ 2016-04-06 15:37 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On 04/06/2016 10:39 AM, Ian Jackson wrote:
> Ross Philipson writes ("[Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block"):
>> It seems the logic is meant to detect sector unaligned buffers for block
>> writes. The NOTing of the logic instead masks off any unaligned bits and
>> also would cause the function to always fail. It seems the function is not
>> used in any of the tools so that is probably why the problem is not seen.
>> In the vhd_read_block function it is correct.
>>
>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
>> ---
>> diff --git a/tools/blktap2/vhd/lib/libvhd.c b/tools/blktap2/vhd/lib/libvhd.c
>> index 1fd5b4e..4ebe012 100644
>> --- a/tools/blktap2/vhd/lib/libvhd.c
>> +++ b/tools/blktap2/vhd/lib/libvhd.c
>> @@ -2188,7 +2188,7 @@ vhd_write_block(vhd_context_t *ctx, uint32_t block, char *data)
>>         if (block >= ctx->bat.entries)
>>                 return -ERANGE;
>>
>> -       if ((unsigned long)data & ~(VHD_SECTOR_SIZE -1))
>> +       if ((unsigned long)data & (VHD_SECTOR_SIZE -1))
> 
> Notwithstanding the discussion in the rest of the thread, this is a
> clearly-correct bugfix to the code in tree, so I have queued it.
> 
> However, I had to do so rather manually because something had mangled
> the whitespace.  Ross, can you check your patch submission path,
> please ?
> 
> Ian.
> 

Never mind, I found the problem. Tabs got turned into spaces. Sorry about that.

-- 
Ross Philipson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-03-29 11:26     ` George Dunlap
  2016-03-30 14:47       ` Ross Philipson
@ 2016-04-06 16:51       ` Ross Philipson
  2016-07-06 13:16         ` George Dunlap
  1 sibling, 1 reply; 12+ messages in thread
From: Ross Philipson @ 2016-04-06 16:51 UTC (permalink / raw)
  To: George Dunlap; +Cc: Wei Liu, xen-devel

On 03/29/2016 07:26 AM, George Dunlap wrote:
> On Fri, Mar 25, 2016 at 4:34 PM, Ross Philipson
> <ross.philipson@gmail.com> wrote:
>> On 03/25/2016 12:11 PM, Wei Liu wrote:
>>> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>>>> It seems the logic is meant to detect sector unaligned buffers for block
>>>> writes. The NOTing of the logic instead masks off any unaligned bits and
>>>> also would cause the function to always fail. It seems the function is not
>>>> used in any of the tools so that is probably why the problem is not seen.
>>>> In the vhd_read_block function it is correct.
>>>>
>>>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
>>>
>>> This seems to fall under tools umbrella. I've look at blktap2 code,
>>> the reasoning is convincing to me so:
>>>
>>>   Acked-by: Wei Liu <wei.liu2@citrix.com>
>>>
>>> But I've never used blktap2 and we don't normally test it so I would
>>> also wait a bit longer to see if anybody objects to this change.
>>>
>>> OOI if no code was using this, how did you discover this problem?
>>
>> We have an old fork of blktap2 from way back when. I was working to extract our
>> changes and turn them into patches so we could rebase on upstream blktap2.
>> Someone long ago found that - I have no idea how but it was a valid fix so I
>> upstreamed it.
>>
>> There are a number of other ones that were found that are still in upstream
>> blktap2 - I plan to send them too.
> 
> Ross,
> 
> Instead of cross-porting fixes from XenServer's blktap3 to the
> bitrotting blktap2, would you consider taking up my work on replacing
> blktap2 with building blktap3 as an external project?

George, a few questions below to get started...

> 
> The basic shape of things that need to happen for that are:
> 
> 1. Allow qemu to provide emulated devices for disks which use hotplug
> scripts (just checked in last week)
> 
> 2. Do the tweaks necessary to allow blktap3 to be built as an
> independent project (outside the XenServer build system)

I am not sure where the latest and greatest blktap3 work can be found. I have
found some old repos that have activity up until the work stopped on that
project in 2013. Can you give me some pointers on this?

Also it seems the overall idea is to not have blktap3 (or any blktap) live in
the Xen tree but rather be external. Where would blktap3 live?

> 
> 3. Switch libxl to use the already-in-tree block-tap script (which
> calls tapctl) rather than linking against the blktap library.

I believe I found this work:

http://lists.xenproject.org/archives/html/xen-devel/2015-07/msg00805.html

It looks like 4 of the set made it in and patches 3 & 5 still need work? I can
re-base them.

> 
> 4. Add a blktap target to raisin.
> 
> #1 is done and was just checked in last week.  #2 shouldn't be a
> terribly large amount of work.  For #3, I've posted patches already,
> and I can probably do a quick rebase for you to pick up if you wanted.
> #4 shouldn't be too hard once you have an out-of-tree build working; I
> can help with that as well.
> 
> If we then add tests for upstream blktap to osstest, then we'll catch
> any breakages, and automatically get updates; and we can delete the
> bitrotting blktap2 out of the tree.
> 
> What do you think?
> 
>  -George
> 


-- 
Ross Philipson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-04-06 15:37   ` Ross Philipson
@ 2016-04-07 16:37     ` Ian Jackson
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2016-04-07 16:37 UTC (permalink / raw)
  To: Ross Philipson; +Cc: xen-devel

Ross Philipson writes ("Re: [Xen-devel] [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block"):
> On 04/06/2016 10:39 AM, Ian Jackson wrote:
> > However, I had to do so rather manually because something had mangled
> > the whitespace.  Ross, can you check your patch submission path,
> > please ?
> 
> Never mind, I found the problem. Tabs got turned into spaces. Sorry about that.

NP, thanks for investigating.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block
  2016-04-06 16:51       ` Ross Philipson
@ 2016-07-06 13:16         ` George Dunlap
  0 siblings, 0 replies; 12+ messages in thread
From: George Dunlap @ 2016-07-06 13:16 UTC (permalink / raw)
  To: Ross Philipson; +Cc: Wei Liu, xen-devel

On Wed, Apr 6, 2016 at 5:51 PM, Ross Philipson <ross.philipson@gmail.com> wrote:
> On 03/29/2016 07:26 AM, George Dunlap wrote:
>> On Fri, Mar 25, 2016 at 4:34 PM, Ross Philipson
>> <ross.philipson@gmail.com> wrote:
>>> On 03/25/2016 12:11 PM, Wei Liu wrote:
>>>> On Thu, Mar 17, 2016 at 06:10:59PM -0400, Ross Philipson wrote:
>>>>> It seems the logic is meant to detect sector unaligned buffers for block
>>>>> writes. The NOTing of the logic instead masks off any unaligned bits and
>>>>> also would cause the function to always fail. It seems the function is not
>>>>> used in any of the tools so that is probably why the problem is not seen.
>>>>> In the vhd_read_block function it is correct.
>>>>>
>>>>> Signed-off-by: Ross Philipson <ross.philipson@ainfosec.com>
>>>>
>>>> This seems to fall under tools umbrella. I've look at blktap2 code,
>>>> the reasoning is convincing to me so:
>>>>
>>>>   Acked-by: Wei Liu <wei.liu2@citrix.com>
>>>>
>>>> But I've never used blktap2 and we don't normally test it so I would
>>>> also wait a bit longer to see if anybody objects to this change.
>>>>
>>>> OOI if no code was using this, how did you discover this problem?
>>>
>>> We have an old fork of blktap2 from way back when. I was working to extract our
>>> changes and turn them into patches so we could rebase on upstream blktap2.
>>> Someone long ago found that - I have no idea how but it was a valid fix so I
>>> upstreamed it.
>>>
>>> There are a number of other ones that were found that are still in upstream
>>> blktap2 - I plan to send them too.
>>
>> Ross,
>>
>> Instead of cross-porting fixes from XenServer's blktap3 to the
>> bitrotting blktap2, would you consider taking up my work on replacing
>> blktap2 with building blktap3 as an external project?
>
> George, a few questions below to get started...
>
>>
>> The basic shape of things that need to happen for that are:
>>
>> 1. Allow qemu to provide emulated devices for disks which use hotplug
>> scripts (just checked in last week)
>>
>> 2. Do the tweaks necessary to allow blktap3 to be built as an
>> independent project (outside the XenServer build system)
>
> I am not sure where the latest and greatest blktap3 work can be found. I have
> found some old repos that have activity up until the work stopped on that
> project in 2013. Can you give me some pointers on this?
>
> Also it seems the overall idea is to not have blktap3 (or any blktap) live in
> the Xen tree but rather be external. Where would blktap3 live?
>
>>
>> 3. Switch libxl to use the already-in-tree block-tap script (which
>> calls tapctl) rather than linking against the blktap library.
>
> I believe I found this work:
>
> http://lists.xenproject.org/archives/html/xen-devel/2015-07/msg00805.html
>
> It looks like 4 of the set made it in and patches 3 & 5 still need work? I can
> re-base them.

Hey Ross,

Sorry for dropping this on the floor -- are you still interested in
this work?  If so I'll take some time to summarize where I got to. :-)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-07-06 13:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-17 22:10 [PATCH] blktap2: Invalid logic detecting unaligned buffers in vhd_write_block Ross Philipson
2016-03-25 16:11 ` Wei Liu
2016-03-25 16:34   ` Ross Philipson
2016-03-25 16:38     ` Wei Liu
2016-03-29 11:26     ` George Dunlap
2016-03-30 14:47       ` Ross Philipson
2016-04-06 16:51       ` Ross Philipson
2016-07-06 13:16         ` George Dunlap
2016-04-06 14:39 ` Ian Jackson
2016-04-06 15:25   ` Ross Philipson
2016-04-06 15:37   ` Ross Philipson
2016-04-07 16:37     ` Ian Jackson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).