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

Bug with DeviceIoControl function call on Windows #16553

Closed
wants to merge 1 commit into from

Conversation

yaroslavmamrokha
Copy link

@yaroslavmamrokha yaroslavmamrokha commented Feb 25, 2020

What does this PR do?

PR is fixing issue with DISKIO ioCounter function, as it is constantly failing with error "The parameter is incorrect" during DeviceIoControl call on Windows environment.

Issue is reproducible on both Windows 7/ Windows 10 x32 Other Windows platforms are not tested.

Changed:

  • Added padding to diskPerformance structure to make it aligned with WinAPI DISK_PERFORMANCE structure. GoLang is not performing alignment and pass structure as is (84 bytes), but WinAPI DeviceIoControl expects aligned structure(88 bytes)

Why is it important?

Currently DiskIO functionality for ioCounter is not working, and produce a lot of errors during run.

Checklist

  • My code follows the style guidelines of this project
    - [ ] I have commented my code, particularly in hard-to-understand areas
    - [ ] I have made corresponding changes to the documentation
    - [ ] I have made corresponding change to the default configuration files
    - [] I have added tests that prove my fix is effective or that my feature works

Author's Checklist

How to test this PR locally

Run metricbeats service and configure it to get DiskIO counters. Check if no error occurs and returned data is valid

Related issues

Use cases

Screenshots

Logs

diskPerformance is not same size as DISK_PERFORMANCE in WinAPI code. In general members order and size is correct, but GoLang is not including structure alignment. Due to this calls of DeviceIoControl in ioCounter are always failing with error "The parameter is incorrect".
Fix is to add in the end of structure missing alignment bytes. In WinAPI code, structure is aligned base on biggest size member. This is LARGE_INTEGER(8 bytes), order of members allows to perform alignment only once, in the end of structure. Sum of all members sizes is 84 bytes, therefore to be aligned structure needs additional 4 bytes.
@yaroslavmamrokha yaroslavmamrokha requested a review from a team as a code owner February 25, 2020 12:10
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Feb 25, 2020

💚 CLA has been signed

@ChrsMark ChrsMark added bug Team:Integrations Label for the Integrations team Team:Services (Deprecated) Label for the former Integrations-Services team labels Feb 26, 2020
@narph narph self-assigned this Feb 26, 2020
@narph
Copy link
Contributor

narph commented Feb 26, 2020

hi @yaroslavmamrokha , I have been having a hard time reproducing this issue, tested on windows 10 (64 bit), Windows Server 2016 (64 bit) and Windows Server 2012 (32 bit).
You mentioned in the discuss ticket that you have this issue starting 7.2 ( the moment we introduced the DeviceIoControl function) so we expected to have other reports from users similar to yours, which I cannot find.
I have also verified the size of the diskPerformance struct and it is 88 when testing.
Can you help us further reproduce this scenario:

  • can you provide us the metricbeat config file?
  • are you running metricbeat as a service?
  • can you provide us more information on the machine you are running windows on (virtualized? , capacity..)

@yaroslavmamrokha
Copy link
Author

yaroslavmamrokha commented Feb 26, 2020

hi @yaroslavmamrokha , I have been having a hard time reproducing this issue, tested on windows 10 (64 bit), Windows Server 2016 (64 bit) and Windows Server 2012 (32 bit).
You mentioned in the discuss ticket that you have this issue starting 7.2 ( the moment we introduced the DeviceIoControl function) so we expected to have other reports from users similar to yours, which I cannot find.
I have also verified the size of the diskPerformance struct and it is 88 when testing.
Can you help us further reproduce this scenario:

  • can you provide us the metricbeat config file?
  • are you running metricbeat as a service?
  • can you provide us more information on the machine you are running windows on (virtualized? , capacity..)

Hi @narph , my apologize...

I've double checked and seems like its x32 only issue. When I've been building x32 project I forgot to change afterwards GOARCH to amd64, and therefore both x32 and x64 Windows was working with x32 app.

This need x32 build

Here is sample of code that i've generated from your source, that is failing:
image

and here same code but x64 version:
image

Attaching code snipped.
test.txt

As for questions:

  • are you running metricbeat as a service? - Yes, installed and running in automatic mode
  • can you provide us more information on the machine you are running windows on (virtualized? , capacity..) - Service itself is installed on Windows 10 x32 IoT version (1809) and also on Windows 7 Embedded x32. Virtualization is disabled. Memory capacity (RAM) 4 Gb. Disk capacity: SSD 120 Gb, divided in 2 drives.
  • can you provide us the metricbeat config file? - Unfortunately I cannot. It should work in base config, just with enabled diskio

@narph
Copy link
Contributor

narph commented Feb 27, 2020

hi @yaroslavmamrokha , thanks for getting back and confirming, I will close the PR, if you have any questions feel free to reach out.

@narph narph closed this Feb 27, 2020
@zube zube bot added [zube]: Done and removed [zube]: Inbox labels Feb 27, 2020
@yaroslavmamrokha
Copy link
Author

yaroslavmamrokha commented Feb 27, 2020

hi @yaroslavmamrokha , thanks for getting back and confirming, I will close the PR, if you have any questions feel free to reach out.

Hi @narph,
So will this issue be addressed later on? As x32 version of metricbeats service is not getting disk performance counters. I've tried up to 10 machines that are using Win 10 IoT x32 or Win7 Embedded x32, and they are failing to get data from DeviceIoControl function.

@narph
Copy link
Contributor

narph commented Feb 27, 2020

@yaroslavmamrokha , I think I misread your previous reply.
I suggest creating a separate issue (else I can do that as well) and linking this PR to it as well in order for us to investigate further.

@yaroslavmamrokha
Copy link
Author

@yaroslavmamrokha , I think I misread your previous reply.
I suggest creating a separate issue (else I can do that as well) and linking this PR to it as well in order for us to investigate further.

Hi @narph
Thanks for suggestion! I've opened new bug, and linked this pull request to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Team:Integrations Label for the Integrations team Team:Services (Deprecated) Label for the former Integrations-Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants