GoogleCloudPlatform / guest-configs

Apache License 2.0
31 stars 41 forks source link

Replace xxd with dd for google_nvme_id #56

Closed ravanelli closed 1 year ago

ravanelli commented 1 year ago

Replace xxd with dd for google_nvme_id google_nvme_id script currently uses xxd to parse nvme device info, but we need to install additional package xxd for fedora, vim-common and vim-filesystem for centos (or rhel) before using it. Replace it with dd and we do not need to install additional packages.

See https://github.com/coreos/fedora-coreos-config/pull/2412 (comment)

We initially tried to replace it with cut, creating a different result than expected. See: https://github.com/GoogleCloudPlatform/guest-configs/pull/49 Discussion about the use of ddvs cut.

Tests for Fedora CoreOS:

nvme id-ns -b /dev/nvme0n1 | xxd -p --seek 384 | xxd -p -r | od -x
0000000 227b 6564 6976 6563 6e5f 6d61 2265 223a
0000020 6570 7372 7369 6574 746e 642d 7369 2d6b
0000040 2230 222c 6964 6b73 745f 7079 2265 223a
0000060 4550 5352 5349 4554 544e 7d22 0000 0000
0000100 0000 0000 0000 0000 0000 0000 0000 0000
*
0007200

nvme id-ns -b /dev/nvme0n1 | dd bs=1 skip=384 2>/dev/null  | od -x
0000000 227b 6564 6976 6563 6e5f 6d61 2265 223a
0000020 6570 7372 7369 6574 746e 642d 7369 2d6b
0000040 2230 222c 6964 6b73 745f 7079 2265 223a
0000060 4550 5352 5349 4554 544e 7d22 0000 0000
0000100 0000 0000 0000 0000 0000 0000 0000 0000
*
0007200

Tests for Debian 12

nvme id-ns -b /dev/nvme0n1 | dd bs=1 skip=384 2>/dev/null  | od -x
0000000 227b 6564 6976 6563 6e5f 6d61 2265 223a
0000020 6f6c 6163 2d6c 766e 656d 732d 6473 312d
0000040 2c22 6422 7369 5f6b 7974 6570 3a22 4c22
0000060 434f 4c41 535f 4453 7d22 0000 0000 0000
0000100 0000 0000 0000 0000 0000 0000 0000 0000
*
0007200

nvme id-ns -b /dev/nvme0n1 | dd bs=1 skip=384 2>/dev/null  | od -x
0000000 227b 6564 6976 6563 6e5f 6d61 2265 223a
0000020 6f6c 6163 2d6c 766e 656d 732d 6473 312d
0000040 2c22 6422 7369 5f6b 7974 6570 3a22 4c22
0000060 434f 4c41 535f 4453 7d22 0000 0000 0000
0000100 0000 0000 0000 0000 0000 0000 0000 0000
*
0007200
google-oss-prow[bot] commented 1 year ago

Hi @ravanelli. Thanks for your PR.

I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
ravanelli commented 1 year ago

@tytso Thanks for you suggestion in https://github.com/GoogleCloudPlatform/guest-configs/pull/49#issuecomment-1648963321. Would you mind a review to double check it?

google-oss-prow[bot] commented 1 year ago

@HuijingHei: changing LGTM is restricted to collaborators

In response to [this](https://github.com/GoogleCloudPlatform/guest-configs/pull/56#pullrequestreview-1549907474): >LGTM Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
google-oss-prow[bot] commented 1 year ago

@travier: changing LGTM is restricted to collaborators

In response to [this](https://github.com/GoogleCloudPlatform/guest-configs/pull/56#pullrequestreview-1551825655): > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
dorileo commented 1 year ago

/lgtm

google-oss-prow[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HuijingHei, ravanelli, travier, zmarano

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/GoogleCloudPlatform/guest-configs/blob/master/OWNERS)~~ [zmarano] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment