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

update model field and add serial field #8200

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smgrol
Copy link

@smgrol smgrol commented Nov 23, 2023

This PR contain two changes:

  1. changed way to obtain information about the model of disk
  2. added a new field for getting serial number of physical disks

Previous version of getting info about model of disk not contain full info about connected disks.
For example, (prev. version)
SELECT name, model FROM block_devices WHERE block_size != '';
image

For inventarization purpose this info about model is not full, because by this info we can't understand which type of disk or which size available on it. Also we can't collect info about serial numbers of connected disks.

For resolve this situation we created this PR .

For example, (new version)

SELECT name, model, serial FROM block_devices WHERE block_size != '';

disk

@smgrol smgrol requested review from a team as code owners November 23, 2023 16:47
Copy link

linux-foundation-easycla bot commented Nov 23, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@zwass
Copy link
Member

zwass commented Nov 27, 2023

@smgrol Thank you for this contribution. Please resolve the EasyCLA issue when you can, as this is required for us to be able to merge code to osquery.

@smgrol smgrol force-pushed the update_block_devices_table branch 2 times, most recently from e9aaa37 to 9638779 Compare November 28, 2023 07:17
@smgrol
Copy link
Author

smgrol commented Nov 28, 2023

@zwass thank you, done

@directionless
Copy link
Member

@Micah-Kolide I know you're in the process of looking at improving this table. Does this fit okay?

@Micah-Kolide
Copy link
Contributor

Micah-Kolide commented Nov 28, 2023

@Micah-Kolide I know you're in the process of looking at improving this table. Does this fit okay?

I don't see any issue with including the serial, and updating the model info. I know where these are on sysfs, so I can look at putting those into what I'm working on as well. These just come from the bus host, which is easy enough to follow.

@smgrol
Copy link
Author

smgrol commented Nov 29, 2023

what a pity that I was not aware that work was underway on the block_devices table
does that mean my PR is not relevant?

@Micah-Kolide
Copy link
Contributor

what a pity that I was not aware that work was underway on the block_devices table does that mean my PR is not relevant?

I still think it's very relevant. The serial is a great addition, and looking at the database model makes sense. The only detractor between this and the work I'm in the middle of is that I was removing libudev completely, which would remove the model database id lookup as well.

I'm still working on getting my PR up, but I'd definitely wait to see how others weigh in.

@smgrol
Copy link
Author

smgrol commented Dec 8, 2023

maybe you can check my PR ?
seems like this info is needed for some people #7980

as I understand it, it is not yet known when your changes will be published, so maybe you can check and apply the changes to what is now?

@concorde18
Copy link

Hello, everyone!
This PR is very relevant for my organisation purposes, so would you kindly start the code-review? @Micah-Kolide or @zwass

@directionless
Copy link
Member

Hi @smgrol and @concorde18 As I understand things, this PR does two things. First, it adds the serial number to the the table. Second, it changes how model is defined, to supplement the information through a udev table.

While those seem straight forward, there is a related PR up at #8182 which removes udev from the implementation. The desire to remove udev, is two fold. Foremost, it allows better performance because of how traversing the child/parent relationships works. Second, it removes udev.

However, one concern I have, is that while it's easy to grab the serial number in 8182, the model mapping is a bit harder. But I also don't understand what that's adding.

What do y'all think?

@concorde18
Copy link

concorde18 commented Dec 24, 2023

Hello @directionless
Thanks for your reply!
And special thanks to @smgrol for this PR, here are simple changes with obvious profit for inventory service (eg cmdb, glpi).
Did i understand correctly - are you offering to rewrite this PR according to changes in #8182, cuz there is a conflict between #8200 and #8182?
And another question - is osquery team gonna to accept #8182?

@smgrol
Copy link
Author

smgrol commented Dec 25, 2023

Hi @directionless !
I decided to publish this PR because a neighboring team wanted to see the full name of the model for a detailed compilation of the inventory database. This question is relevant for large teams and companies. Using osquery it is easy to solve without any additional complications.

Complete information about the model allows you to reveal its properties in more detail (for example, KINGSTON SEDC500 can be either 480GB or 960GB). This information is much easier to transfer without additional processing operations to neighboring teams.

I think this data would also be relevant for other users.

So, what can I do so that as a result we can see the full name of the disk model?

Hi @concorde18! thanks for joining the question

@directionless directionless added this to the 5.12.0 milestone Jan 7, 2024
@directionless directionless modified the milestones: 5.12.0, 5.13 Feb 29, 2024
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

Successfully merging this pull request may close these issues.

None yet

5 participants