* [PATCH] xfsdump: Revert dump version bump for 32bit projid fix
@ 2012-10-22 20:24 Eric Sandeen
2012-10-22 22:31 ` Dave Chinner
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Eric Sandeen @ 2012-10-22 20:24 UTC (permalink / raw)
To: xfs-oss
commit 1e309da7a4f7e2a2f456bf6b7cea4c5f1181cd36 fixed xfsdump to
properly save & restore the top 16 bits of a 32-bit projid, which
otherwise was being dropped (and restored as 0) in older xfsdump.
The original thought was to bump the dump version, so that we know
whether the dump (may) have the top 16 bits filled in. In practice
this would prevent older restores from restoring newer dumps, and
losing the top 16 bits contained in these newer dumps.
However, in hindsight this appears to be of limited value. I
propose that the dump version change is unuseful/unwanted for a
couple reasons:
* There is no actual dump *format* change; the structure size
is the same, and the top 16 bits were properly zeroed before; old
restores will read these fixed dumps without problems and without
restoring garbage. IOW, they will behave exactly as buggily as
they did before. And worst case, if a dump containing the top 16
bits is mangled by an old restore, this can be easily remedied by
simply re-restoring with updated userspace.
* We have no reliable method to know whether 32 bit project IDs are
in use; the feature flag was not added to the GEOM call at the time
of implementation. Therefore we cannot reliably bump to V4 only
for projid32bit filesystems, and we cannot restrict V4 restores
only to projid32bit filesystems. So the dump version is not
useful for feature cross-checking purposes.
I spoke with wkendall via email, and although he may not have given
it the most scrutiny (having moved on from xfsdump-land), he also
felt that a version dump may not be warranted.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/common/global.c b/common/global.c
index 1793ff4..8e49d8b 100644
--- a/common/global.c
+++ b/common/global.c
@@ -281,7 +281,6 @@ global_version_check( u_int32_t version )
case GLOBAL_HDR_VERSION_1:
case GLOBAL_HDR_VERSION_2:
case GLOBAL_HDR_VERSION_3:
- case GLOBAL_HDR_VERSION_4:
return BOOL_TRUE;
default:
return BOOL_FALSE;
diff --git a/common/global.h b/common/global.h
index 5138ed8..6556a68 100644
--- a/common/global.h
+++ b/common/global.h
@@ -28,15 +28,13 @@
#define GLOBAL_HDR_VERSION_1 1
#define GLOBAL_HDR_VERSION_2 2
#define GLOBAL_HDR_VERSION_3 3
-#define GLOBAL_HDR_VERSION_4 4
- /* version 4 adds 32-bit projid (projid_hi)
- * version 3 uses the full 32-bit inode generation number in direnthdr_t.
+ /* version 3 uses the full 32-bit inode generation number in direnthdr_t.
* version 2 adds encoding of holes and a change to on-tape inventory format.
* version 1 adds extended file attribute dumping.
* version 0 xfsrestore can't handle media produced
* by version 1 xfsdump.
*/
-#define GLOBAL_HDR_VERSION GLOBAL_HDR_VERSION_4
+#define GLOBAL_HDR_VERSION GLOBAL_HDR_VERSION_3
#define GLOBAL_HDR_STRING_SZ 0x100
#define GLOBAL_HDR_TIME_SZ 4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xfsdump: Revert dump version bump for 32bit projid fix
2012-10-22 20:24 [PATCH] xfsdump: Revert dump version bump for 32bit projid fix Eric Sandeen
@ 2012-10-22 22:31 ` Dave Chinner
2012-10-23 0:05 ` Eric Sandeen
2012-10-23 19:51 ` Ben Myers
2012-10-26 20:31 ` Ben Myers
2 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2012-10-22 22:31 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
On Mon, Oct 22, 2012 at 03:24:16PM -0500, Eric Sandeen wrote:
> commit 1e309da7a4f7e2a2f456bf6b7cea4c5f1181cd36 fixed xfsdump to
> properly save & restore the top 16 bits of a 32-bit projid, which
> otherwise was being dropped (and restored as 0) in older xfsdump.
>
> The original thought was to bump the dump version, so that we know
> whether the dump (may) have the top 16 bits filled in. In practice
> this would prevent older restores from restoring newer dumps, and
> losing the top 16 bits contained in these newer dumps.
>
> However, in hindsight this appears to be of limited value. I
> propose that the dump version change is unuseful/unwanted for a
> couple reasons:
>
> * There is no actual dump *format* change; the structure size
> is the same, and the top 16 bits were properly zeroed before; old
> restores will read these fixed dumps without problems and without
> restoring garbage. IOW, they will behave exactly as buggily as
> they did before. And worst case, if a dump containing the top 16
> bits is mangled by an old restore, this can be easily remedied by
> simply re-restoring with updated userspace.
>
> * We have no reliable method to know whether 32 bit project IDs are
> in use; the feature flag was not added to the GEOM call at the time
> of implementation. Therefore we cannot reliably bump to V4 only
> for projid32bit filesystems, and we cannot restrict V4 restores
> only to projid32bit filesystems. So the dump version is not
> useful for feature cross-checking purposes.
Seems reasonable. Definitely solves most of the nasty problems
you've been juggling caused by a version bump and 32bit projid
detection. Do we have an xfstest that verifies these assumptions are
valid?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfsdump: Revert dump version bump for 32bit projid fix
2012-10-22 22:31 ` Dave Chinner
@ 2012-10-23 0:05 ` Eric Sandeen
0 siblings, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2012-10-23 0:05 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs-oss
On 10/22/12 5:31 PM, Dave Chinner wrote:
> On Mon, Oct 22, 2012 at 03:24:16PM -0500, Eric Sandeen wrote:
>> commit 1e309da7a4f7e2a2f456bf6b7cea4c5f1181cd36 fixed xfsdump to
>> properly save & restore the top 16 bits of a 32-bit projid, which
>> otherwise was being dropped (and restored as 0) in older xfsdump.
>>
>> The original thought was to bump the dump version, so that we know
>> whether the dump (may) have the top 16 bits filled in. In practice
>> this would prevent older restores from restoring newer dumps, and
>> losing the top 16 bits contained in these newer dumps.
>>
>> However, in hindsight this appears to be of limited value. I
>> propose that the dump version change is unuseful/unwanted for a
>> couple reasons:
>>
>> * There is no actual dump *format* change; the structure size
>> is the same, and the top 16 bits were properly zeroed before; old
>> restores will read these fixed dumps without problems and without
>> restoring garbage. IOW, they will behave exactly as buggily as
>> they did before. And worst case, if a dump containing the top 16
>> bits is mangled by an old restore, this can be easily remedied by
>> simply re-restoring with updated userspace.
>>
>> * We have no reliable method to know whether 32 bit project IDs are
>> in use; the feature flag was not added to the GEOM call at the time
>> of implementation. Therefore we cannot reliably bump to V4 only
>> for projid32bit filesystems, and we cannot restrict V4 restores
>> only to projid32bit filesystems. So the dump version is not
>> useful for feature cross-checking purposes.
>
> Seems reasonable. Definitely solves most of the nasty problems
> you've been juggling caused by a version bump and 32bit projid
> detection. Do we have an xfstest that verifies these assumptions are
> valid?
I'll whip up a test that tests what happens when restoring a dump
w/ all 32 bits to a filesystem w/o the feature.
I'm not sure testing a restoration of an "old" dump to a 32-bit
projid is too useful, it really just tests that the zeroed bits
in the old dump image stay zeroed.
-Eric
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfsdump: Revert dump version bump for 32bit projid fix
2012-10-22 20:24 [PATCH] xfsdump: Revert dump version bump for 32bit projid fix Eric Sandeen
2012-10-22 22:31 ` Dave Chinner
@ 2012-10-23 19:51 ` Ben Myers
2012-10-26 20:31 ` Ben Myers
2 siblings, 0 replies; 5+ messages in thread
From: Ben Myers @ 2012-10-23 19:51 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
Hey Eric,
On Mon, Oct 22, 2012 at 03:24:16PM -0500, Eric Sandeen wrote:
> commit 1e309da7a4f7e2a2f456bf6b7cea4c5f1181cd36 fixed xfsdump to
> properly save & restore the top 16 bits of a 32-bit projid, which
> otherwise was being dropped (and restored as 0) in older xfsdump.
>
> The original thought was to bump the dump version, so that we know
> whether the dump (may) have the top 16 bits filled in. In practice
> this would prevent older restores from restoring newer dumps, and
> losing the top 16 bits contained in these newer dumps.
>
> However, in hindsight this appears to be of limited value. I
> propose that the dump version change is unuseful/unwanted for a
> couple reasons:
>
> * There is no actual dump *format* change; the structure size
> is the same, and the top 16 bits were properly zeroed before; old
> restores will read these fixed dumps without problems and without
> restoring garbage. IOW, they will behave exactly as buggily as
> they did before. And worst case, if a dump containing the top 16
> bits is mangled by an old restore, this can be easily remedied by
> simply re-restoring with updated userspace.
>
> * We have no reliable method to know whether 32 bit project IDs are
> in use; the feature flag was not added to the GEOM call at the time
> of implementation. Therefore we cannot reliably bump to V4 only
> for projid32bit filesystems, and we cannot restrict V4 restores
> only to projid32bit filesystems. So the dump version is not
> useful for feature cross-checking purposes.
>
> I spoke with wkendall via email, and although he may not have given
> it the most scrutiny (having moved on from xfsdump-land), he also
> felt that a version dump may not be warranted.
This looks fine to me. I still think that bumping the format version was
probably the right thing to do. But in this case it's not so important that
I'm inclined to push harder for it. It was good to understand some of the
implications of doing that, so maybe next time we do need to bump the version
we'll be ready.
Warning the user when he could be losing the top 16 bits of the project id is
something for the would-be-nice list.
Reviewed-by: Ben Myers <bpm@sgi.com>
Regards,
Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfsdump: Revert dump version bump for 32bit projid fix
2012-10-22 20:24 [PATCH] xfsdump: Revert dump version bump for 32bit projid fix Eric Sandeen
2012-10-22 22:31 ` Dave Chinner
2012-10-23 19:51 ` Ben Myers
@ 2012-10-26 20:31 ` Ben Myers
2 siblings, 0 replies; 5+ messages in thread
From: Ben Myers @ 2012-10-26 20:31 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
Hey Eric,
On Mon, Oct 22, 2012 at 03:24:16PM -0500, Eric Sandeen wrote:
> commit 1e309da7a4f7e2a2f456bf6b7cea4c5f1181cd36 fixed xfsdump to
> properly save & restore the top 16 bits of a 32-bit projid, which
> otherwise was being dropped (and restored as 0) in older xfsdump.
>
> The original thought was to bump the dump version, so that we know
> whether the dump (may) have the top 16 bits filled in. In practice
> this would prevent older restores from restoring newer dumps, and
> losing the top 16 bits contained in these newer dumps.
>
> However, in hindsight this appears to be of limited value. I
> propose that the dump version change is unuseful/unwanted for a
> couple reasons:
>
> * There is no actual dump *format* change; the structure size
> is the same, and the top 16 bits were properly zeroed before; old
> restores will read these fixed dumps without problems and without
> restoring garbage. IOW, they will behave exactly as buggily as
> they did before. And worst case, if a dump containing the top 16
> bits is mangled by an old restore, this can be easily remedied by
> simply re-restoring with updated userspace.
>
> * We have no reliable method to know whether 32 bit project IDs are
> in use; the feature flag was not added to the GEOM call at the time
> of implementation. Therefore we cannot reliably bump to V4 only
> for projid32bit filesystems, and we cannot restrict V4 restores
> only to projid32bit filesystems. So the dump version is not
> useful for feature cross-checking purposes.
>
> I spoke with wkendall via email, and although he may not have given
> it the most scrutiny (having moved on from xfsdump-land), he also
> felt that a version dump may not be warranted.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Pulled this into git://oss.sgi.com/xfs/cmds/xfsdump.git, master branch.
Regards,
Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-10-26 20:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-22 20:24 [PATCH] xfsdump: Revert dump version bump for 32bit projid fix Eric Sandeen
2012-10-22 22:31 ` Dave Chinner
2012-10-23 0:05 ` Eric Sandeen
2012-10-23 19:51 ` Ben Myers
2012-10-26 20:31 ` Ben Myers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.