Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
@ 2019-09-06  0:50 Anand Jain
  2019-09-06  7:21 ` Nikolay Borisov
  0 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2019-09-06  0:50 UTC (permalink / raw)
  To: linux-btrfs

It's common to copy/snapshot an OS image to run another instance of the OS.
A duplicate fsid can't be mounted on the same system unless the fsid is
changed by using btrfstune -m.

However in some circumstances the image needs to go back to the original
fsid /metadata_uuid.

As of now btrfstune -M fails if the specified uuid isn't unique, as show
below.

btrfstune -M $(btrfs in dump-super ./2-2g.img | grep metadata_uuid | \
					awk '{print $2}') ./2-2g.img
ERROR: fsid 87f8d9c5-a8b7-438e-a890-17bbe11c95e5 is not unique

But as we are changing the fsid of an unmounted image, so its ok to
leave it to the users choice if the fsid is not unique, so that the
image can be sent back the system where it was used with that fsid.

So this patch drops the check test_uuid_unique() for btrfstune -M|m.
Thanks.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfstune.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/btrfstune.c b/btrfstune.c
index afa3aae35412..4befcadef8b1 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -570,10 +570,6 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 			error("could not parse UUID: %s", new_fsid_str);
 			return 1;
 		}
-		if (!test_uuid_unique(new_fsid_str)) {
-			error("fsid %s is not unique", new_fsid_str);
-			return 1;
-		}
 	}
 
 	fd = open(device, O_RDWR);
-- 
2.21.0 (Apple Git-120)


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

* Re: [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
  2019-09-06  0:50 [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M Anand Jain
@ 2019-09-06  7:21 ` Nikolay Borisov
  2019-09-06  9:27   ` Anand Jain
  0 siblings, 1 reply; 7+ messages in thread
From: Nikolay Borisov @ 2019-09-06  7:21 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On 6.09.19 г. 3:50 ч., Anand Jain wrote:
> It's common to copy/snapshot an OS image to run another instance of the OS.
> A duplicate fsid can't be mounted on the same system unless the fsid is
> changed by using btrfstune -m.
> 
> However in some circumstances the image needs to go back to the original
> fsid /metadata_uuid.
> 
> As of now btrfstune -M fails if the specified uuid isn't unique, as show
> below.
> 
> btrfstune -M $(btrfs in dump-super ./2-2g.img | grep metadata_uuid | \
> 					awk '{print $2}') ./2-2g.img
> ERROR: fsid 87f8d9c5-a8b7-438e-a890-17bbe11c95e5 is not unique

NAK.

This is intended. Otherwise it's an open avenue for the user to shoot
themselves in the foot. If you know what you are doing and are
absolutely sure the original fs is no longer present - then just flush
libblkid cache and you'll be able to set the FSID back to the original one.




> 
> But as we are changing the fsid of an unmounted image, so its ok to
> leave it to the users choice if the fsid is not unique, so that the
> image can be sent back the system where it was used with that fsid.
> 
> So this patch drops the check test_uuid_unique() for btrfstune -M|m.
> Thanks.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  btrfstune.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/btrfstune.c b/btrfstune.c
> index afa3aae35412..4befcadef8b1 100644
> --- a/btrfstune.c
> +++ b/btrfstune.c
> @@ -570,10 +570,6 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>  			error("could not parse UUID: %s", new_fsid_str);
>  			return 1;
>  		}
> -		if (!test_uuid_unique(new_fsid_str)) {
> -			error("fsid %s is not unique", new_fsid_str);
> -			return 1;
> -		}
>  	}
>  
>  	fd = open(device, O_RDWR);
> 

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

* Re: [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
  2019-09-06  7:21 ` Nikolay Borisov
@ 2019-09-06  9:27   ` Anand Jain
  2019-09-09 11:40     ` Nikolay Borisov
  0 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2019-09-06  9:27 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs


<snip>

> This is intended. Otherwise it's an open avenue for the user to shoot
> themselves in the foot.

  I don't understand how?

> If you know what you are doing and are
> absolutely sure the original fs is no longer present 
> - then just flush
> libblkid cache and you'll be able to set the FSID back to the original one.
> 
<snip>

  No no its not about the stale cache holding the original fsid. The use
  case is - a golden copy of the bootable OS image is being used and
  shares the same fsid on multiple hosts.
  Now if you want to mount another copy it for some changes, you need to
  btrfstune -m on the copy. And later if you want to boot it
  successfully, it needs its original fsid back.

HTH, Anand

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

* Re: [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
  2019-09-06  9:27   ` Anand Jain
@ 2019-09-09 11:40     ` Nikolay Borisov
  2019-09-10  5:12       ` Anand Jain
  0 siblings, 1 reply; 7+ messages in thread
From: Nikolay Borisov @ 2019-09-09 11:40 UTC (permalink / raw)
  To: Anand Jain, Nikolay Borisov, linux-btrfs



On 6.09.19 г. 12:27 ч., Anand Jain wrote:
> 
> <snip>
> 
>> This is intended. Otherwise it's an open avenue for the user to shoot
>> themselves in the foot.
> 
>  I don't understand how?
> 
>> If you know what you are doing and are
>> absolutely sure the original fs is no longer present - then just flush
>> libblkid cache and you'll be able to set the FSID back to the original
>> one.
>>
> <snip>
> 
>  No no its not about the stale cache holding the original fsid. The use

Then this is worst. UUID, by definition, are Unique. What you want to is
to toss the Unique part, meaning we'll really be left with some sort of
ID. What's more - the UUID is used by libblkid to populate the available
devices so not making it unique can cause subtle bugs.

>  case is - a golden copy of the bootable OS image is being used and
>  shares the same fsid on multiple hosts.
>  Now if you want to mount another copy it for some changes, you need to
>  btrfstune -m on the copy. And later if you want to boot it
>  successfully, it needs its original fsid back.
> 
> HTH, Anand

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

* Re: [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
  2019-09-09 11:40     ` Nikolay Borisov
@ 2019-09-10  5:12       ` Anand Jain
  2019-09-11 17:01         ` David Sterba
  0 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2019-09-10  5:12 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs


Nikolay,

>>> This is intended. Otherwise it's an open avenue for the user to shoot
>>> themselves in the foot.
>>
>>   I don't understand how?

  Again. Any idea how? Is there any test case?

  <snip>

> UUID, by definition, are Unique. What you want to is
> to toss the Unique part, meaning we'll really be left with some sort of
> ID. What's more - the UUID is used by libblkid to populate the available
> devices so not making it unique can cause subtle bugs.

- Irrespective of the fstype, when fs image file is copied/snapshot-ed
   the fsid is indeed going to be duplicate.

- xfs and btrfs kernel doesn't allow duplicate fsid to be mounted which
   is fair and accepted. (I remember fixing the duplicate fsid bugs in
   the kernel, just fyi if you are concerned about them).

- ext4 kernel does allow duplicate fsid to be mounted.

- btrfstume -M <uuid> isn't the place to check if the fsid is a
   duplicate. Because, libblkid will be unaware of the complete list of
   fs images with its fsid.

- As I said before, its a genuine use-case here where the user wants to
   revert the fsid change, so that btrfs fs root image can be booted.
   Its up-to the user if fsid is duplicate in the user space, as btrfs
   kernel rightly fails the mount if its duplicate fsid anyways.

Thanks, Anand

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

* Re: [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
  2019-09-10  5:12       ` Anand Jain
@ 2019-09-11 17:01         ` David Sterba
  2019-09-12  0:45           ` Anand Jain
  0 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2019-09-11 17:01 UTC (permalink / raw)
  To: Anand Jain; +Cc: Nikolay Borisov, linux-btrfs

On Tue, Sep 10, 2019 at 01:12:43PM +0800, Anand Jain wrote:
> 
> Nikolay,
> 
> >>> This is intended. Otherwise it's an open avenue for the user to shoot
> >>> themselves in the foot.
> >>
> >>   I don't understand how?
> 
>   Again. Any idea how? Is there any test case?
> 
>   <snip>
> 
> > UUID, by definition, are Unique. What you want to is
> > to toss the Unique part, meaning we'll really be left with some sort of
> > ID. What's more - the UUID is used by libblkid to populate the available
> > devices so not making it unique can cause subtle bugs.
> 
> - Irrespective of the fstype, when fs image file is copied/snapshot-ed
>    the fsid is indeed going to be duplicate.
> 
> - xfs and btrfs kernel doesn't allow duplicate fsid to be mounted which
>    is fair and accepted. (I remember fixing the duplicate fsid bugs in
>    the kernel, just fyi if you are concerned about them).
> 
> - ext4 kernel does allow duplicate fsid to be mounted.
> 
> - btrfstume -M <uuid> isn't the place to check if the fsid is a
>    duplicate. Because, libblkid will be unaware of the complete list of
>    fs images with its fsid.

I don't understand this part. Blkid tracks the device iformation, like
the uuid, and the cache gets updated. So what does 'will be unaware of
the complete list' mean?

If it's on the same host it's a matter of keeping the cache in sync with
the actual devices.

> - As I said before, its a genuine use-case here where the user wants to
>    revert the fsid change, so that btrfs fs root image can be booted.
>    Its up-to the user if fsid is duplicate in the user space, as btrfs
>    kernel rightly fails the mount if its duplicate fsid anyways.

Reverting the uuid is fine and requiring the uuids to be unique is
preventing the users doing stupid things unknowingly. You seem to have a
usecase where even duplicate uuids are required but I'm afraid it's not
all clear how is it supposed to work. A few more examples or commands
would be helpful.

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

* Re: [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M
  2019-09-11 17:01         ` David Sterba
@ 2019-09-12  0:45           ` Anand Jain
  0 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2019-09-12  0:45 UTC (permalink / raw)
  To: dsterba, Nikolay Borisov, linux-btrfs


thanks for the comments, more inline below.

>> - btrfstume -M <uuid> isn't the place to check if the fsid is a
>>     duplicate. Because, libblkid will be unaware of the complete list of
>>     fs images with its fsid.
> 
> I don't understand this part. Blkid tracks the device iformation, like
> the uuid, and the cache gets updated. So what does 'will be unaware of
> the complete list' mean?
> 
> If it's on the same host it's a matter of keeping the cache in sync with
> the actual devices.

In case of vm guest images copied from the golden image there is no
physical device or loop device or nbd device until its configured on
the host when required, so check for duplicate fsid at the time of
btrfstune -M is not convincing or a very limited approach.

>> - As I said before, its a genuine use-case here where the user wants to
>>     revert the fsid change, so that btrfs fs root image can be booted.
>>     Its up-to the user if fsid is duplicate in the user space, as btrfs
>>     kernel rightly fails the mount if its duplicate fsid anyways.
> 
> Reverting the uuid is fine 

ok thanks.

> and requiring the uuids to be unique is
> preventing the users doing stupid things unknowingly.

Right it should be done. But..
btrfstune -M is a wrong place. Because it can't avoid all the
cases of fsid getting duplicated.
Even after btrfstune -M, the fsid can be duplicated by the user.
So what's the point in restricting the btrfstune -M and fail to
undo the changed fsid.

> You seem to have a
> usecase where even duplicate uuids are required but I'm afraid it's not
> all clear how is it supposed to work. A few more examples or commands
> would be helpful.
> 

In the use case here, even the host is also running a copy of the golden
image (same fsid as vm guest) and because of duplicate fsid you can
only mount a vm guest image on the host after the btrfstune -m command
on the vm guest image. But after you have done that, as the vm guest
fsid is changed, it fails to boot, unfortunately changed fsid can not
be undone without this patch.

The fsid can be duplicate by many different other ways anyways. So in
this case how does it matter if btrfstune -M tries to ensure that fsid
is unique among the blkid known set of devices, which may change any
time after btrfstune -M as well (just copy a vm guest and map it to
a loop device). So btrfstune -M should be free from this check and
help the use case as above.

And if we are concerned about the duplicate fsid as I asked Nikolay
before, we need to know what are problems in specifies, so that it can
be fixed in separate patches, but definitely not in btrfstune -M as
it can't fix the duplicate fsid problem completely as vm images can
be copied and mapped to a loop/nbd device anytime even after
btrfstune -M.

Thanks, Anand

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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06  0:50 [PATCH] btrfs-progs: drop unique uuid test for btrfstune -M Anand Jain
2019-09-06  7:21 ` Nikolay Borisov
2019-09-06  9:27   ` Anand Jain
2019-09-09 11:40     ` Nikolay Borisov
2019-09-10  5:12       ` Anand Jain
2019-09-11 17:01         ` David Sterba
2019-09-12  0:45           ` Anand Jain

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox