Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lsblk: Would it be bad to not ignore /dev/sr* if their size is 0 ? #1118

Closed
scdbackup opened this issue Aug 8, 2020 · 12 comments
Closed

lsblk: Would it be bad to not ignore /dev/sr* if their size is 0 ? #1118

scdbackup opened this issue Aug 8, 2020 · 12 comments

Comments

@scdbackup
Copy link

Hi,

i am fiddling with the kernel to improve its perception of optical drives
and media. One of my experiments is to set the size of /dev/sr0 to 0 if
a unwritten and unformatted disc is inserted.

But now sr0 with size 0 does not show up in lsblk.
The program gets to see the device but ignores it:

  $ export LSBLK_DEBUG=all
  $ lsblk /dev/sr0
  1227: lsblk:      CXT: [0x7fff3f564c20]: setting context for sr0 [parent=(nil), wholedisk=(nil)]
  1227: lsblk:      CXT: [0x7fff3f564c20]: sr0: filename=/dev/sr0
  1227: lsblk:      CXT: [0x7fff3f564c20]: zero size device -- ignore
  1227: lsblk:      CXT: [0x7fff3f564c20]: reset
  $

Reason is obviously this code snippet from
https://github.com/karelzak/util-linux/blob/master/misc-utils/lsblk.c
line 1205

        /* Ignore devices of zero size */
        if (!lsblk->all_devices && dev->size == 0) {
                DBG(DEV, ul_debugobj(dev, "zero size device -- ignore"));
                return -1;
        }

By a mere glitch in drivers/scsi/sr.c:get_sectorsize() blank media currently
get attributed a size of 2048 bytes. SCSI command READ CAPACITY cannot
reply -1 and get_sectorsize() naively makes cd->capacity = 1 out of its
reply 0.
Actually one should not ask READ CAPACITY if READ DISC INFORMATION replies
Disc Type 0 (= Empty, aka Blank).

I wonder whether it would be possible to get an exemption for /dev/sr*,
which allows them to be listed despite size 0.
Is there a strong reason why size 0 would cause trouble ?

(An empty drive gets reported with size 1024M, which i would try to
change to 0, too, if no reasons against that pop up.
)


Own experiments with lsblk:

I cloned
git://git.kernel.org/pub/scm/utils/util-linux/util-linux.git
and added a clause to the Ignore-devices test which prevents dev->filename
"/dev/sr[0-9]" from getting ignored.

The run did not explode, at least:

  NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
  sr0   11:0    1       0 rom

But "SIZE" is empty instead of 0.
So i added my not-/dev/sr[0-9] clause to this test in line 909:

        case COL_SIZE:
                if (!dev->size)
                        break;

Now the output is:

  NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
  sr0   11:0    1   0B  0 rom

Would this be bad in any way ?


Have a nice day :)

Thomas

@scdbackup
Copy link
Author

Hi,

i now stumbled over lsblk option -a.
So in some way my question is answered as: "It is not harmful".

Still i deem my chances bad for getting a change into the kernel which
causes /dev/sr0 to vanish from lsblk without -a and displays an empty
SIZE text with -a.

So at least the test in line 909 should get the same !lsblk->all_devices
as the test in line 1206.

And in the sum i would still wish for empty /dev/sr0 to be listed without -a
because it is traditionally listed without -a, albeit with a wrong size.

Have a nice day :)

Thomas

@karelzak
Copy link
Collaborator

I think about an extension to the --include option. Now --include filter is used as "what is specified and nothing else". It would be nice to add semantic "default devices and devices specified by --include +list", it means:

  $ lsblk --include 11    # only sr* devices
  $ lsblk --include +11   # all default devices and sr*

(11 is a major number). It means +MAJ will override all other tests (like size==0).

So, you can add shell alias lsblk="lsblk --include +11" to see what you need.

@scdbackup
Copy link
Author

Hi,

meanwhile i got used to adding -a when asking for the currently known size
of the medium.
In the beginning it was a surprise to see them missing and i thought my
change had broken util-linux. (Which would be very embarrassing if proposed
in public ...)

Do you agree that blank media and empty tray should be represented by the
kernel as having 0 size ?

If there is any reason for the current habit to let blank media appear
with 2 KiB, and empty tray appear with the size of the last readable medium,
then i fail to imagine. (After boot with empty tray, the size is 2 KiB less
than 1024 MiB.)

Have a nice day :)

Thomas

@scdbackup
Copy link
Author

Hi,

i forgot to ask:

Is there a special reason why SIZE 0 is shown by lsblk as empty text ?
If so, how about an option to show it as "0" or "0B" ?
Just as help for gestures like

expr $(lsblk -a -b -n -o SIZE /dev/sr1) / 2048

(It's not so easy to get the size of empty tray otherwise, because already
open(2) fails.)

Have a nice day :)

Thomas

@karelzak
Copy link
Collaborator

karelzak commented Sep 9, 2020

I think the current "empty" device test is pretty basic (=poor). It would be better to analyze the device and create some rules what we need to display.

For example -- the original idea has been to filter out the loop devices, but it would be better to check if the device is associated with any file than make the decision on the device size.

Ad empty SIZE -- we usually use an empty string when util is not able to determine the value, but this is not the case. You're probably right, this should be fixed.

karelzak added a commit that referenced this issue Sep 9, 2020
Addresses: #1118
Signed-off-by: Karel Zak <kzak@redhat.com>
karelzak added a commit that referenced this issue Sep 9, 2020
* do not ignore all empty devices, we need more smart solution

* ignore only loop devices without backing file, for example:
 # touch img
 # losetup -f img
 losetup: img: Warning: file is smaller than 512 bytes; the loop device may be useless or invisible for system tools.

 - old version display nothing
 - new version:

 # lsblk /dev/loop0
 NAME  MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
 loop0   7:0    0   0B  0 loop

Addresses: #1118
Signed-off-by: Karel Zak <kzak@redhat.com>
@karelzak
Copy link
Collaborator

karelzak commented Sep 9, 2020

Implemented. Test it for your use-case. Thanks for your report!

@karelzak karelzak closed this as completed Sep 9, 2020
@scdbackup
Copy link
Author

Hi,

thanks for the changes.

I currently have my first kernel patch set pending (about automatic tray
loading). If it succeeds i will next submit patches to attribute size 0
to empty tray and blank medium. Building util-linux from source and
testing it with my change proposal is now on my todo list.

Have a nice day :)

Thomas

@scdbackup
Copy link
Author

Hi,

i forgot that i already had a working util-linux git clone from a month
ago.
But after "git pull" and "make" it does not show empty sr drives with
my hacked kernel 4.19.

Reason is a logical flaw in ignore_empty(). It lets pass devices with
non-zero size and loop devices with backing. But /dev/sr0 with empty
tray in my hacked kernel 4.19 is neither.

I could make it work by this change:

diff --git a/misc-utils/lsblk.c b/misc-utils/lsblk.c
index 82eae6bad..c69b33d1e 100644
--- a/misc-utils/lsblk.c
+++ b/misc-utils/lsblk.c
@@ -1154,7 +1154,9 @@ static int ignore_empty(struct lsblk_device *dev)
 {
        if (dev->size != 0)
                return 0;
-       if (dev->maj == LOOPDEV_MAJOR && loopdev_has_backing_file(dev->filename))
+       if (dev->maj != LOOPDEV_MAJOR)
+               return 0;
+       if (loopdev_has_backing_file(dev->filename))
                return 0;
        return 1;
 }

Results:

$ ./lsblk /dev/sr*
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
sr0   11:0    1   0B  0 rom
sr1   11:1    1   0B  0 rom
$ ./lsblk -b -n -o SIZE /dev/sr0
   0

Have a nice day :)

Thomas

@scdbackup
Copy link
Author

Hi,

i forgot to report that a loop device with backing still shows up
after my change:
'''
$ ls /dev/loop[0-9]
/dev/loop0 /dev/loop2 /dev/loop4 /dev/loop6
/dev/loop1 /dev/loop3 /dev/loop5 /dev/loop7
$ ./lsblk /dev/loop[0-9]
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
loop1 7:1 0 21.5K 0 loop
$ sudo losetup -d /dev/loop1
$ ./lsblk /dev/loop[0-9]
$ touch empty_file
$ sudo losetup /dev/loop1 empty_file
losetup: /home/thomas/x: Warning: file is smaller than 512 bytes; the loop device may be useless or invisible for system tools.
$ ./lsblk /dev/loop[0-9]
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
loop1 7:1 0 0B 0 loop
$
'''

Have a nice day :)

Thomas

@scdbackup
Copy link
Author

Sorry: A copy+paste mishap (s//home/thomas/x/empty_file/) and the wrong type of ''' marks.
I see no opportunity to edit or delete the previous post.

karelzak added a commit that referenced this issue Sep 9, 2020
This patch improves the previous commit to accept also another empty devices.

Addresses: #1118
Signed-off-by: Karel Zak <kzak@redhat.com>
@karelzak
Copy link
Collaborator

karelzak commented Sep 9, 2020

Ah, good point. I have only extended the current behavior to show empty loopdev, it's useless for your use-case. Sorry ;-)

Fixed (I hope).

@scdbackup
Copy link
Author

Hi,

the new change works for me. Thanks again.

loop devices show up with 0 or more bytes, but not if not backed.
(Tested on 4.19.118 and 5.9-rc2-next-20200827.)

sr devices on my hacked kernel 4.19.118 show up with size 0 if the tray
is not loaded, or empty, or if a blank CD-RW is inserted. (And still with
non-zero size if a readable medium is in.)

Now i only have to convince linux-scsi of my ideas ...

Have a nice day :)

Thomas

karelzak added a commit that referenced this issue Nov 13, 2020
Addresses: #1118
Signed-off-by: Karel Zak <kzak@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants