| Message ID | 1450371968-27997-4-git-send-email-ian.jackson@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2015-12-17 at 17:06 +0000, Ian Jackson wrote: > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> > --- > mg-schema-test-database | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/mg-schema-test-database b/mg-schema-test-database > index a4cb732..4e0ee68 100755 > --- a/mg-schema-test-database > +++ b/mg-schema-test-database > @@ -448,12 +448,22 @@ END > done > > # As we copy, we note everything we're not borrowing as > - # belonging to the parent db. > + # belonging to the parent db. We borrow shares of a shared > + # resource. If we borrow only some rather than all of the > + # shares, neither DB will be able to unshare it. This is what actually happens, rather than this comment explaining a thing we are avoiding, right? I ask because although the text (and the use of "properly" in the subject) seems pretty clear it's the former I was surprised to find this code was borrowing partial shares and therefore setting up such a situation. Given the caveat below would it not be better to just never allow this? I suppose that given the list of tasks to borrow came from the user this might be considered a "keep both pieces" situation. > + > + # In principle it might be possible to actually use different > + # shares of the same resource with different dbs. However the > + # `sharetype' contains the osstest revision, which prevents > + # sharing between test and real versions of osstest code. > + > cat >>$t.import <<END > $(make_xdbref_task $maindbname 'not borrowed' '' PARENT) > UPDATE resources > SET owntaskid = $(taskid xdbref $maindbname) > - WHERE owntaskid != $(borrowtaskid $task); > + WHERE owntaskid != $(borrowtaskid $task) > + AND owntaskid != $(taskid magic shared) > + AND owntaskid != $(taskid magic preparing); > COMMIT; > END >
Ian Campbell writes ("Re: [OSSTEST PATCH 4/9] mg-schema-test-database: Borrow shares properly"):
> On Thu, 2015-12-17 at 17:06 +0000, Ian Jackson wrote:
> > # As we copy, we note everything we're not borrowing as
> > - # belonging to the parent db.
> > + # belonging to the parent db. We borrow shares of a shared
> > + # resource. If we borrow only some rather than all of the
> > + # shares, neither DB will be able to unshare it.
>
> This is what actually happens, rather than this comment explaining a thing
> we are avoiding, right?
Yes.
> I ask because although the text (and the use of "properly" in the subject)
> seems pretty clear it's the former I was surprised to find this code was
> borrowing partial shares and therefore setting up such a situation. Given
> the caveat below would it not be better to just never allow this?
I have added this to the commit message:
Previously, the test database would be generated in a broken state:
resources share-host/foo/{1,2,...} exist but the resource host/foo/0
is allocated to magic/xdbref rather than to magic/shared. This causes
various resource allocation machinery to crash. (Even if the host is
entirely un-borrowed.)
I guess it might be possible for mg-schema-test-database to check that
either none or all of the shares of a host are being borrowed, but I
don't think it's worth it. The osstest git hash in the sharetype will
in practice mostly prevent undesirable sharing of the same resource
between test and real db instances.
> I suppose that given the list of tasks to borrow came from the user
> this might be considered a "keep both pieces" situation.
Yes. This partial sharing can only affect you if you have allocated
individual shares to the task you say you want to borrow from.
Ian.
On Thu, 2015-12-17 at 17:43 +0000, Ian Jackson wrote: > Ian Campbell writes ("Re: [OSSTEST PATCH 4/9] mg-schema-test > -database: Borrow shares properly"): > > On Thu, 2015-12-17 at 17:06 +0000, Ian Jackson wrote: > > > # As we copy, we note everything we're not borrowing as > > > - # belonging to the parent db. > > > + # belonging to the parent db. We borrow shares of a > shared > > > + # resource. If we borrow only some rather than all of > the > > > + # shares, neither DB will be able to unshare it. > > > > This is what actually happens, rather than this comment explaining > a thing > > we are avoiding, right? > > Yes. > > > I ask because although the text (and the use of "properly" in the subject) > > seems pretty clear it's the former I was surprised to find this code was > > borrowing partial shares and therefore setting up such a situation. Given > > the caveat below would it not be better to just never allow this? > > I have added this to the commit message: > > Previously, the test database would be generated in a broken state: > resources share-host/foo/{1,2,...} exist but the resource host/foo/0 > is allocated to magic/xdbref rather than to magic/shared. This causes > various resource allocation machinery to crash. (Even if the host is > entirely un-borrowed.) > > I guess it might be possible for mg-schema-test-database to check that > either none or all of the shares of a host are being borrowed, but I > don't think it's worth it. The osstest git hash in the sharetype will > in practice mostly prevent undesirable sharing of the same resource > between test and real db instances. > > > I suppose that given the list of tasks to borrow came from the user > > this might be considered a "keep both pieces" situation. > > Yes. This partial sharing can only affect you if you have allocated > individual shares to the task you say you want to borrow from. Grand, with the extra commit log bit from above: Acked-by: Ian Campbell <ian.campbell@citrix.com>
diff --git a/mg-schema-test-database b/mg-schema-test-database index a4cb732..4e0ee68 100755 --- a/mg-schema-test-database +++ b/mg-schema-test-database @@ -448,12 +448,22 @@ END done # As we copy, we note everything we're not borrowing as - # belonging to the parent db. + # belonging to the parent db. We borrow shares of a shared + # resource. If we borrow only some rather than all of the + # shares, neither DB will be able to unshare it. + + # In principle it might be possible to actually use different + # shares of the same resource with different dbs. However the + # `sharetype' contains the osstest revision, which prevents + # sharing between test and real versions of osstest code. + cat >>$t.import <<END $(make_xdbref_task $maindbname 'not borrowed' '' PARENT) UPDATE resources SET owntaskid = $(taskid xdbref $maindbname) - WHERE owntaskid != $(borrowtaskid $task); + WHERE owntaskid != $(borrowtaskid $task) + AND owntaskid != $(taskid magic shared) + AND owntaskid != $(taskid magic preparing); COMMIT; END
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> --- mg-schema-test-database | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)