| Message ID | cfc78e5581e0142caf9a5fe049213c8e5de90efa.1398786620.git.dsterba@suse.cz (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | David Sterba |
| Headers | show |
On 29 April 2014 17:02, David Sterba <dsterba@suse.cz> wrote: > The entire device size may not be available to the filesystem, eg. if > it's modified via resize. Print this information if it can be obtained > from the DEV_INFO ioctl. > > Print the device ID on the same line as the device name and move size to > the next line. > > Sample: > /dev/sda7, ID: 3 > Device size: 10.00GiB > FS occuppied: 5.00GiB Spelling mistake. s/occuppied/occupied/. > Data,RAID10: 512.00MiB > Metadata,RAID10: 512.00MiB > System,RAID10: 4.00MiB > Unallocated: 9.00GiB > > Signed-off-by: David Sterba <dsterba@suse.cz> > --- > cmds-device.c | 6 +++--- > cmds-fi-disk_usage.c | 13 ++++++++++++- > cmds-fi-disk_usage.h | 6 +++++- > 3 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/cmds-device.c b/cmds-device.c > index 7a9d808b36dd..519725f83e8c 100644 > --- a/cmds-device.c > +++ b/cmds-device.c > @@ -447,9 +447,9 @@ static int _cmd_device_usage(int fd, char *path, int mode) > } > > for (i = 0; i < device_info_count; i++) { > - printf("%s\t%10s\n", device_info_ptr[i].path, > - df_pretty_sizes(device_info_ptr[i].size, mode)); > - > + printf("%s, ID: %llu\n", device_info_ptr[i].path, > + device_info_ptr[i].devid); > + print_device_sizes(fd, &device_info_ptr[i], mode); > print_device_chunks(fd, device_info_ptr[i].devid, > device_info_ptr[i].size, > info_ptr, info_count, > diff --git a/cmds-fi-disk_usage.c b/cmds-fi-disk_usage.c > index 067c60078710..ddb064cc4c66 100644 > --- a/cmds-fi-disk_usage.c > +++ b/cmds-fi-disk_usage.c > @@ -499,7 +499,8 @@ int load_device_info(int fd, struct device_info **device_info_ptr, > > info[ndevs].devid = dev_info.devid; > strcpy(info[ndevs].path, (char *)dev_info.path); > - info[ndevs].size = get_partition_size((char *)dev_info.path); > + info[ndevs].device_size = get_partition_size((char *)dev_info.path); > + info[ndevs].size = dev_info.total_size; > ++ndevs; > } > > @@ -879,5 +880,15 @@ void print_device_chunks(int fd, u64 devid, u64 total_size, > printf(" Unallocated: %*s%10s\n", > (int)(20 - strlen("Unallocated")), "", > df_pretty_sizes(total_size - allocated, mode)); > +} > > +void print_device_sizes(int fd, struct device_info *devinfo, int mode) > +{ > + printf(" Device size: %*s%10s\n", > + (int)(20 - strlen("Device size")), "", > + df_pretty_sizes(devinfo->device_size, mode)); > + printf(" FS occuppied:%*s%10s\n", Here too. s/occuppied/occupied/. > + (int)(20 - strlen("FS occupied")), "", > + df_pretty_sizes(devinfo->size, mode)); > + } > } > diff --git a/cmds-fi-disk_usage.h b/cmds-fi-disk_usage.h > index 787b4eb56acf..79cc2a115bc5 100644 > --- a/cmds-fi-disk_usage.h > +++ b/cmds-fi-disk_usage.h > @@ -27,7 +27,10 @@ int cmd_filesystem_usage(int argc, char **argv); > struct device_info { > u64 devid; > char path[BTRFS_DEVICE_PATH_NAME_MAX]; > - u64 size; > + /* Size of the block device */ > + u64 device_size; > + /* Size that's occupied by the filesystem, can be changed via resize */ > + u64 size; > }; > > /* > @@ -50,5 +53,6 @@ char *df_pretty_sizes(u64 size, int mode); > void print_device_chunks(int fd, u64 devid, u64 total_size, > struct chunk_info *chunks_info_ptr, > int chunks_info_count, int mode); > +void print_device_sizes(int fd, struct device_info *devinfo, int mode); > > #endif > -- > 1.9.0 Same spelling mistake (occuppied) also occurs in the following patches too: [PATCH 08/14] btrfs-progs: compare unallocated space against the correct value [PATCH 12/14] btrfs-progs: replace df_pretty_sizes with pretty_size_mode Thanks, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I David thanks, to take care of these enhancements. On 04/29/2014 09:23 PM, Mike Fleetwood wrote: > On 29 April 2014 17:02, David Sterba <dsterba@suse.cz> wrote: >> The entire device size may not be available to the filesystem, eg. if >> it's modified via resize. Print this information if it can be obtained >> from the DEV_INFO ioctl. >> >> Print the device ID on the same line as the device name and move size to >> the next line. >> >> Sample: >> /dev/sda7, ID: 3 >> Device size: 10.00GiB >> FS occuppied: 5.00GiB > > Spelling mistake. s/occuppied/occupied/. I found a bit unclear the "FS occupied" terms. Can I suggest "Resized to:" instead of "FS occupied:", and to show it only when the two values differ ? In fact this value has a meaning only if a filesystem is resized. BR G.Baroncelli > >> Data,RAID10: 512.00MiB >> Metadata,RAID10: 512.00MiB >> System,RAID10: 4.00MiB >> Unallocated: 9.00GiB >> >> Signed-off-by: David Sterba <dsterba@suse.cz> >> --- >> cmds-device.c | 6 +++--- >> cmds-fi-disk_usage.c | 13 ++++++++++++- >> cmds-fi-disk_usage.h | 6 +++++- >> 3 files changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/cmds-device.c b/cmds-device.c >> index 7a9d808b36dd..519725f83e8c 100644 >> --- a/cmds-device.c >> +++ b/cmds-device.c >> @@ -447,9 +447,9 @@ static int _cmd_device_usage(int fd, char *path, int mode) >> } >> >> for (i = 0; i < device_info_count; i++) { >> - printf("%s\t%10s\n", device_info_ptr[i].path, >> - df_pretty_sizes(device_info_ptr[i].size, mode)); >> - >> + printf("%s, ID: %llu\n", device_info_ptr[i].path, >> + device_info_ptr[i].devid); >> + print_device_sizes(fd, &device_info_ptr[i], mode); >> print_device_chunks(fd, device_info_ptr[i].devid, >> device_info_ptr[i].size, >> info_ptr, info_count, >> diff --git a/cmds-fi-disk_usage.c b/cmds-fi-disk_usage.c >> index 067c60078710..ddb064cc4c66 100644 >> --- a/cmds-fi-disk_usage.c >> +++ b/cmds-fi-disk_usage.c >> @@ -499,7 +499,8 @@ int load_device_info(int fd, struct device_info **device_info_ptr, >> >> info[ndevs].devid = dev_info.devid; >> strcpy(info[ndevs].path, (char *)dev_info.path); >> - info[ndevs].size = get_partition_size((char *)dev_info.path); >> + info[ndevs].device_size = get_partition_size((char *)dev_info.path); >> + info[ndevs].size = dev_info.total_size; >> ++ndevs; >> } >> >> @@ -879,5 +880,15 @@ void print_device_chunks(int fd, u64 devid, u64 total_size, >> printf(" Unallocated: %*s%10s\n", >> (int)(20 - strlen("Unallocated")), "", >> df_pretty_sizes(total_size - allocated, mode)); >> +} >> >> +void print_device_sizes(int fd, struct device_info *devinfo, int mode) >> +{ >> + printf(" Device size: %*s%10s\n", >> + (int)(20 - strlen("Device size")), "", >> + df_pretty_sizes(devinfo->device_size, mode)); >> + printf(" FS occuppied:%*s%10s\n", > > Here too. s/occuppied/occupied/. > >> + (int)(20 - strlen("FS occupied")), "", >> + df_pretty_sizes(devinfo->size, mode)); >> + } >> } >> diff --git a/cmds-fi-disk_usage.h b/cmds-fi-disk_usage.h >> index 787b4eb56acf..79cc2a115bc5 100644 >> --- a/cmds-fi-disk_usage.h >> +++ b/cmds-fi-disk_usage.h >> @@ -27,7 +27,10 @@ int cmd_filesystem_usage(int argc, char **argv); >> struct device_info { >> u64 devid; >> char path[BTRFS_DEVICE_PATH_NAME_MAX]; >> - u64 size; >> + /* Size of the block device */ >> + u64 device_size; >> + /* Size that's occupied by the filesystem, can be changed via resize */ >> + u64 size; >> }; >> >> /* >> @@ -50,5 +53,6 @@ char *df_pretty_sizes(u64 size, int mode); >> void print_device_chunks(int fd, u64 devid, u64 total_size, >> struct chunk_info *chunks_info_ptr, >> int chunks_info_count, int mode); >> +void print_device_sizes(int fd, struct device_info *devinfo, int mode); >> >> #endif >> -- >> 1.9.0 > > Same spelling mistake (occuppied) also occurs in the following patches too: > [PATCH 08/14] btrfs-progs: compare unallocated space against the correct value > [PATCH 12/14] btrfs-progs: replace df_pretty_sizes with pretty_size_mode > > Thanks, > Mike > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Tue, Apr 29, 2014 at 08:23:08PM +0100, Mike Fleetwood wrote: > > /dev/sda7, ID: 3 > > Device size: 10.00GiB > > FS occuppied: 5.00GiB > > Spelling mistake. s/occuppied/occupied/. > > +void print_device_sizes(int fd, struct device_info *devinfo, int mode) > > +{ > > + printf(" Device size: %*s%10s\n", > > + (int)(20 - strlen("Device size")), "", > > + df_pretty_sizes(devinfo->device_size, mode)); > > + printf(" FS occuppied:%*s%10s\n", > > Here too. s/occuppied/occupied/. > > > + (int)(20 - strlen("FS occupied")), "", > > + df_pretty_sizes(devinfo->size, mode)); > > + } > > } > > Same spelling mistake (occuppied) also occurs in the following patches too: > [PATCH 08/14] btrfs-progs: compare unallocated space against the correct value > [PATCH 12/14] btrfs-progs: replace df_pretty_sizes with pretty_size_mode Thanks for catching it. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 30, 2014 at 01:39:27PM +0200, Goffredo Baroncelli wrote: > >> Sample: > >> /dev/sda7, ID: 3 > >> Device size: 10.00GiB > >> FS occuppied: 5.00GiB > > > > Spelling mistake. s/occuppied/occupied/. > > I found a bit unclear the "FS occupied" terms. We're running out of terms to describe and distinguish the space that the filesystem uses. Here's what we have already: - used - allocated - reserved - total 'occupied' seemed like a good choice to me, though it may be not obvious at first. The rationale behind is to denote space that the filesystem can potentially use on the device - in one word. > Can I suggest "Resized to:" instead of "FS occupied:", and to show it > only when the two values differ ? In fact this value has a meaning > only if a filesystem is resized. 'Resized to' sounds good to me and makes sense in the context of the output: /dev/sda6, ID: 2 Device size: 10.00GiB Resized to: 5.00GiB Data,RAID10: 512.00MiB Metadata,RAID10: 512.00MiB System,RAID10: 4.00MiB Unallocated: 9.00GiB The next point, display only if it's different from size. This makes calculating the filesystem size conditional and if eg. I filter the output I can't simply do 'grep -i occupied'. The points against that are that this makes the output longer and the resized devices are probably not the most common scenario. So as a compromise, we can add an option to enable more verbose output. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/04/14 13:11, David Sterba wrote: > On Wed, Apr 30, 2014 at 01:39:27PM +0200, Goffredo Baroncelli wrote: >>>> Sample: >>>> /dev/sda7, ID: 3 >>>> Device size: 10.00GiB >>>> FS occuppied: 5.00GiB >>> >>> Spelling mistake. s/occuppied/occupied/. >> >> I found a bit unclear the "FS occupied" terms. > > We're running out of terms to describe and distinguish the space that > the filesystem uses. > 'occupied' seemed like a good choice to me, though it may be not obvious The space that the filesystem uses in total seems to me is called the "size". It has nothing to do with utilization. /dev/sda6, ID: 2 Device size: 10.00GiB Filesystem size: 5.00GiB and perhaps only show that "Device size" line if it differs from the filesystem size by more than a block. Frank -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 30 Apr 2014, Frank Kingswood wrote: > On 30/04/14 13:11, David Sterba wrote: >> On Wed, Apr 30, 2014 at 01:39:27PM +0200, Goffredo Baroncelli wrote: >>> >>> I found a bit unclear the "FS occupied" terms. >> >> We're running out of terms to describe and distinguish the space that >> the filesystem uses. >> >> 'occupied' seemed like a good choice to me, though it may be not obvious > > The space that the filesystem uses in total seems to me is called the > "size". It has nothing to do with utilization. > > /dev/sda6, ID: 2 > Device size: 10.00GiB > Filesystem size: 5.00GiB FS size was what I was about to suggest, before I saw your reply. It makes more sense to me than 'Occupied' and seems cleaner than 'Resized To'. It sort of mirrors how LVM describes PV / VG / LV sizes, too.
On 04/30/2014 03:37 PM, David Taylor wrote: > On Wed, 30 Apr 2014, Frank Kingswood wrote: >> On 30/04/14 13:11, David Sterba wrote: >>> On Wed, Apr 30, 2014 at 01:39:27PM +0200, Goffredo Baroncelli wrote: >>>> >>>> I found a bit unclear the "FS occupied" terms. >>> >>> We're running out of terms to describe and distinguish the space that >>> the filesystem uses. >>> >>> 'occupied' seemed like a good choice to me, though it may be not obvious >> >> The space that the filesystem uses in total seems to me is called the >> "size". It has nothing to do with utilization. >> >> /dev/sda6, ID: 2 >> Device size: 10.00GiB >> Filesystem size: 5.00GiB > > FS size was what I was about to suggest, before I saw your reply. Pay attention that this value is not the Filesystem size, but to the maximum space the of THE DEVICE the filesystem is allowed to use. The filesystem size (the space available or the sum of the space available and the one occupied) is based on this value but it should be very different (think about a RAID 1 on three device of different sizes....) > > It makes more sense to me than 'Occupied' and seems cleaner than > 'Resized To'. It sort of mirrors how LVM describes PV / VG / LV > sizes, too. >
On Wed, Apr 30, 2014 at 07:38:00PM +0200, Goffredo Baroncelli wrote: > On 04/30/2014 03:37 PM, David Taylor wrote: > > On Wed, 30 Apr 2014, Frank Kingswood wrote: > >> On 30/04/14 13:11, David Sterba wrote: > >>> On Wed, Apr 30, 2014 at 01:39:27PM +0200, Goffredo Baroncelli wrote: > >>>> > >>>> I found a bit unclear the "FS occupied" terms. > >>> > >>> We're running out of terms to describe and distinguish the space that > >>> the filesystem uses. > >>> > >>> 'occupied' seemed like a good choice to me, though it may be not obvious > >> > >> The space that the filesystem uses in total seems to me is called the > >> "size". It has nothing to do with utilization. > >> > >> /dev/sda6, ID: 2 > >> Device size: 10.00GiB > >> Filesystem size: 5.00GiB > > > > FS size was what I was about to suggest, before I saw your reply. > > Pay attention that this value is not the Filesystem size, > but to the maximum space the of THE DEVICE the filesystem is allowed to use. I agree that plain 'Filesystem size' could be misleading, using the same term that has an established meaning can cause misuderstandings in bugreports. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 30, 2014 at 02:37:16PM +0100, David Taylor wrote: > It makes more sense to me than 'Occupied' and seems cleaner than > 'Resized To'. It sort of mirrors how LVM describes PV / VG / LV > sizes, too. Do you have a concrete example how we could map the btrfs sizes based on LVM? -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 30, 2014 at 02:31:18PM +0100, Frank Kingswood wrote: > >>>> Device size: 10.00GiB > >>>> FS occuppied: 5.00GiB > >>> > >>I found a bit unclear the "FS occupied" terms. > > > >We're running out of terms to describe and distinguish the space that > >the filesystem uses. > > >'occupied' seemed like a good choice to me, though it may be not obvious > > The space that the filesystem uses in total seems to me is called the > "size". It has nothing to do with utilization. > > /dev/sda6, ID: 2 > Device size: 10.00GiB > Filesystem size: 5.00GiB > > and perhaps only show that "Device size" line if it differs from the > filesystem size by more than a block. I have noticed that the help text of 'btrfs fi resize' already uses 'occupy': $ btrfs fi resize --help usage: btrfs filesystem resize [devid:][+/-]<newsize>[gkm]|[devid:]max <path> Resize a filesystem If 'max' is passed, the filesystem will occupy all available space on the device 'devid'. First appeared in commit 35401ac1902544637ac26634e139f04ae31c2cbf Author: Goffredo Baroncelli <kreijack@inwind.it> Date: Thu Mar 11 22:32:50 2010 +0100 I've been thinking about all the proposals and I'm not feeling comfortable to pick the winner yet. So, I'll hide the entry from the output for now so the patchset can be pushed next step upstream but does not introduce something that's visible in UI but not agreed on. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-device.c b/cmds-device.c index 7a9d808b36dd..519725f83e8c 100644 --- a/cmds-device.c +++ b/cmds-device.c @@ -447,9 +447,9 @@ static int _cmd_device_usage(int fd, char *path, int mode) } for (i = 0; i < device_info_count; i++) { - printf("%s\t%10s\n", device_info_ptr[i].path, - df_pretty_sizes(device_info_ptr[i].size, mode)); - + printf("%s, ID: %llu\n", device_info_ptr[i].path, + device_info_ptr[i].devid); + print_device_sizes(fd, &device_info_ptr[i], mode); print_device_chunks(fd, device_info_ptr[i].devid, device_info_ptr[i].size, info_ptr, info_count, diff --git a/cmds-fi-disk_usage.c b/cmds-fi-disk_usage.c index 067c60078710..ddb064cc4c66 100644 --- a/cmds-fi-disk_usage.c +++ b/cmds-fi-disk_usage.c @@ -499,7 +499,8 @@ int load_device_info(int fd, struct device_info **device_info_ptr, info[ndevs].devid = dev_info.devid; strcpy(info[ndevs].path, (char *)dev_info.path); - info[ndevs].size = get_partition_size((char *)dev_info.path); + info[ndevs].device_size = get_partition_size((char *)dev_info.path); + info[ndevs].size = dev_info.total_size; ++ndevs; } @@ -879,5 +880,15 @@ void print_device_chunks(int fd, u64 devid, u64 total_size, printf(" Unallocated: %*s%10s\n", (int)(20 - strlen("Unallocated")), "", df_pretty_sizes(total_size - allocated, mode)); +} +void print_device_sizes(int fd, struct device_info *devinfo, int mode) +{ + printf(" Device size: %*s%10s\n", + (int)(20 - strlen("Device size")), "", + df_pretty_sizes(devinfo->device_size, mode)); + printf(" FS occuppied:%*s%10s\n", + (int)(20 - strlen("FS occupied")), "", + df_pretty_sizes(devinfo->size, mode)); + } } diff --git a/cmds-fi-disk_usage.h b/cmds-fi-disk_usage.h index 787b4eb56acf..79cc2a115bc5 100644 --- a/cmds-fi-disk_usage.h +++ b/cmds-fi-disk_usage.h @@ -27,7 +27,10 @@ int cmd_filesystem_usage(int argc, char **argv); struct device_info { u64 devid; char path[BTRFS_DEVICE_PATH_NAME_MAX]; - u64 size; + /* Size of the block device */ + u64 device_size; + /* Size that's occupied by the filesystem, can be changed via resize */ + u64 size; }; /* @@ -50,5 +53,6 @@ char *df_pretty_sizes(u64 size, int mode); void print_device_chunks(int fd, u64 devid, u64 total_size, struct chunk_info *chunks_info_ptr, int chunks_info_count, int mode); +void print_device_sizes(int fd, struct device_info *devinfo, int mode); #endif
The entire device size may not be available to the filesystem, eg. if it's modified via resize. Print this information if it can be obtained from the DEV_INFO ioctl. Print the device ID on the same line as the device name and move size to the next line. Sample: /dev/sda7, ID: 3 Device size: 10.00GiB FS occuppied: 5.00GiB Data,RAID10: 512.00MiB Metadata,RAID10: 512.00MiB System,RAID10: 4.00MiB Unallocated: 9.00GiB Signed-off-by: David Sterba <dsterba@suse.cz> --- cmds-device.c | 6 +++--- cmds-fi-disk_usage.c | 13 ++++++++++++- cmds-fi-disk_usage.h | 6 +++++- 3 files changed, 20 insertions(+), 5 deletions(-)