Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Revert "device: check for existing device PCI path before waiting" #375

Closed
wants to merge 1 commit into from

Conversation

amshinde
Copy link
Member

We should not check for the sysfs path assciated with the PCI address
as the device node may still not be created. Only a uevent with a device
name associated indicates that the device is ready.

This change was also causing the sandbox lock to be not released causing
the uevent listening loop to be locked permanently.

This reverts commit 7caf1c8.

@sboeuf
Copy link

sboeuf commented Sep 19, 2018

@amshinde is this PR solving the issue regarding the inconsistent results when hotplugging some extra network?

@@ -122,15 +122,6 @@ func getPCIDeviceName(s *sandbox, pciID string) (string, error) {
}
}

// Check if the PCI path is present before we setup the pci device map.
_, err = os.Stat(filepath.Join(rootBusPath, pciAddr))
if err == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's missing is s.Unlock() here. Could you please add it here instead of reverting the whole commit? It does fix a situation that is not handled in the original code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bergwolf See my comment in the commit message for this. We should be taking out this check.
For eg, checking for /sys/devices/pci0000:00/0000:00:04.0 is not sufficient, the device is fully attached when virtio# directory under that path (/sys/devices/pci0000:00/0000:00:04.0/virtio3/device). is created which we can only deduce with the uevent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fair enough. I was trying to fix the issue that the device shows up even before kata-agent starts. But I agree the fix is not clean enough. Let's revisit it later on.

@amshinde
Copy link
Member Author

@sboeuf No there are still issues @bergwolf is seeing. Wasnt able to reproduce the issue on my end yesterday.

@amshinde amshinde changed the title wip: Revert "device: check for existing device PCI path before waiting" Revert "device: check for existing device PCI path before waiting" Sep 19, 2018
…ing"

We should not check for the sysfs path assciated with the PCI address
as the device node may still not be created. Only a uevent with a device
name associated indicates that the device is ready.

This change was also causing the sandbox lock to be not released causing
the uevent listening loop to be locked permanently.

This reverts commit 7caf1c8.

Fixes kata-containers#379

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@codecov
Copy link

codecov bot commented Sep 20, 2018

Codecov Report

Merging #375 into master will increase coverage by 1.68%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #375      +/-   ##
==========================================
+ Coverage   45.43%   47.11%   +1.68%     
==========================================
  Files          16       15       -1     
  Lines        3053     2447     -606     
==========================================
- Hits         1387     1153     -234     
+ Misses       1503     1145     -358     
+ Partials      163      149      -14

@gnawux
Copy link
Member

gnawux commented Sep 21, 2018

should we close this PR as we merged #380 ?

@bergwolf
Copy link
Member

The commit is folded into #380 and merged there. See 4163b8b

@bergwolf bergwolf closed this Sep 21, 2018
lifupan added a commit to lifupan/agent that referenced this pull request May 11, 2019
Since runtime needs to update gogo/protobuf to v1.2.1
to support latest shimv2, in order to keep align with
runtime, update it too.

Shortlog since last vendoring of github.com/gogo/protobuf:

    746e99c merged in golang/protobuf commit aa810b61a9c79d51363740d207bb46cf8e620ed5 - proto: fix handling of required fields after multiple violations
    4f863fb merged in golang/protobuf commit 89a0c16f4dc2a70c0ed864d8ef64878f24fdaa51 - README.md: remove usage of group in example
    2f3f4c2 merged in golang/protobuf commit 7d1b268556d691919f2262240737157830eab632 - jsonpb: avoid unexported fields in hand-crafted message (kata-containers#526)
    f2db49f merged in golang/protobuf commit f5983d50c82d70eaa88c17080245cc871558081f - proto: make invalid UTF-8 errors non-fatal (kata-containers#525)
    7aa71d7 merged in golang/protobuf commit 560bdb64431cc123098c2db67f16053a923a0688 - jsonpb: strictly document JSONPBMarshaler and JSONPBUnmarshaler behavior (kata-containers#524)
    eee5829 merged in golang/protobuf commit 93b26e6a70e37abb14f2f88194949312b0592a84 - protoc-gen-go: refactor generator by splitting up generateMessage
    ad62c6c part of golang's 427e165 commit. Missed removing this file with their test refactor
    efb8d72 merged in golang/protobuf commit 427e165155e0a4ff5993a36657c1f733f5b0f782 - proto: fix and cleanup test for deterministic marshal with custom marshalers
    7143b48 merged in golang/protobuf commit 14aad3d5ea4c323bcd7a2137e735da24a76e814c - jsonpb: avoid copying string-valued map-keys
    48e2601 merged in golang/protobuf commit 1325a051a2753cd67556b182843b1b693d0854cd -  proto: fix quadratic behavior in nested map marshaling (no size caching for stdduration/stdtime)
    bc71a26 merged in golang/protobuf commit f05648d464991ab1aa8cf6a499122c56f0f50f2f - jsonpb: handle map key and value properties properly
    be27d1b merged in golang/protobuf commit 9eb2c01ac278a5d89ce4b2be68fe4500955d8179 - jsonpb: decode int32/uint32/float32/float64 strings
    b43a52d merged in golang/protobuf commit 05f48f4eaf0e05663b562bab533cdd472238ce29 - proto: revert UTF-8 validation for proto2
    808c1f7 merged in golang/protobuf commit 64db29d85ff91ba669cfaf009d5f400a4da8a55f - jsonpb: error on scalar enum provided for repeated enums instead of panic
    07eab6a use only one write in the varint writer when possible (kata-containers#504)
    dd51cd8 fix typo independant to independent (kata-containers#512)
    211a54c Add godocs link to Readme.md (kata-containers#506)
    e87afe3 Fix text unmarshal for (u)int(8/16) fields (kata-containers#498)
    e5d5b02 Codegen for well-known types (kata-containers#489)
    6f222ca reorder some of the protoc paths in order to prefer our protobuf/google/protobuf/*.proto files. This is just to avoid using the wrong protos if you have the same protos in you gopath/src dir. (kata-containers#502)
    fd322a3 fix error: bad Go source code was generated, illegal hexadecimal number (kata-containers#488)
    61dbc13 Jsonpb custom type - kata-containers#411 (kata-containers#491)
    e14cafb Customtype  Warnings and issues  update (kata-containers#479)
    7c690ae Exact slice allocation for repeated packed varints (kata-containers#480)
    4aa4cc2 Adding missing func to CustomType documentation (kata-containers#483)
    5669497 bumped the go version (kata-containers#475)
    64d6d2f added nil check in Proto/Size methods fix kata-containers#444 (kata-containers#451)
    3e657e5 fix for letmegrpc (kata-containers#474)
    3cefc55 options to not generate xxx fields (kata-containers#467)
    4c0a09c updated to go1.11 and removed go1.9 (kata-containers#473)
    2b9e95f merged in golang/protobuf commit 70c277a8a150a8e069492e6600926300405c2884 - Fix unmarshaling JSON object with escaped string into Struct type. (kata-containers#464)
    2f2ea5  Added ProtoSize wrapper functions for the well known types  kata-containers#438 (kata-containers#443)
    5440baf exact slice allocation for fixed size packed fields (kata-containers#437)
    6eaa97b Added gRPC Course on Udemy (kata-containers#434)
    a4c2ffc Update Readme.md
    646de4d Fix typo (kata-containers#441)
    636bf03 fix kata-containers#427 consistent import naming between the import declaration and the vars in grpc
    7d68e88 fix build by regenerating everything
    fae8c2d fix git diff for travis
    2c42fe8 merged 7c4add53b497798e7fd7b204f28e41ab409bdbb7 from golang/protobuf. protoc-gen-go: remove deprecated function in grpc (kata-containers#426)
    ebc0565 merged 3fac2a27c94f99f4379551928df388fcb0ad37ce from golang/protobuf. ptypes: optimize Is to avoid prefix scan (kata-containers#425)
    37f19cd Handle deterministic marshaling for generated marshalers in XXX_Marshal
    67fcf76 Swap type assert and fix it (kata-containers#418)
    30cf7ac gofmt
    6b99319 travis: opt into apt get
    110e410 text: allow customtype to have a Bytes() method (kata-containers#227)
    99cb001 dev: amortize cost of growing a Buffer - merge fae8ec697c5d103f717d7fec21103cb5ec020bc8 from golang/protobuf
    1a0e3bd dev: proto: remove unused writeRaw function - merge d167f5cf056d2db6c0f53f44a3309ac60b99ab5b from golang/protobuf
    99bb9bf dev: Revert "protoc-gen-go: use standard library context (requires Go1.9) - merge bf2da8229df5077275b46b301818c6219ebe1003 from golang/protobuf
    9c8b44c dev: Implement "import public" using type aliases. - merge 6fb5325cf9e4b38b58cef6cd1f60c773cc2d5ad2 from golang/protobuf
    6487871 Expose vanity TurnOnGoRegistration (kata-containers#402)
    b8814cc dev: protoc-gen-go: fix generation of proxy getters for distant types - merge 9bb87600c289706cc58f76b46a91b05ddd2a44d8 from golang/protobuf
    9ddc509 dev: protoc-gen-go: fix up generation of package names - merge 3b4abe1a0672c5916c1937b8817dde8aeb579fe5 from golang/protobuf
    cbb7298 dev: protoc-gen-go: revert some API changes - merge 06c268a946d24fdcb0b59370c36ab876ece17556 from golang/protobuf
    8a67e47 dev: proto: do not allow unknown fields to satisfy required field bit - merge 91cccdb44a5fc8dfdf368e8b4d517a21de94dce9 from golang/protobuf
    265a302 dev: protoc-gen-go: Dont rely on local package name for mset name hack - merge 9c8fb7a95075eb047ab75e702de52f68ff360f17 from golang/protobuf
    00f8f1f dev: proto: ignore unknown fields in map entries - merge b028a76c61b7288aefe6746ab7b561d7eb15ab71 from golang/protobuf
    90d0c2a dev: conformance: remove useless variable declaration - merge conformance: remove useless variable declaration from golang/protobuf
    60491a7 dev: conformance: clean up, fix conformance tests - merge ab964bf603354327027b1974c2d1a199ce839899 from golang/protobuf
    9f8212a dev: protoc-gen-go: fix generation of public imports -merge 80c8f764516eebbf17174ea9fd61601d6a52f0f6 from golang/protobuf
    3860157 dev: Reduce a bunch of generated code in oneof sizers. - merge d0dc0def2e8a155b703a9b4966ca8f803ce06308 from golang/protobuf
    e41f35d protoc-gen-go: dont generate blank // import comment - merge b244a785444d0c500df2e0c6b968c05531365a00 from golang/protobuf
    6764c01 dev: golden_test: normalize path separators for Windows - merge 12a586e0adaf626e5d2f8da7881f321f076dbe2c from golang/protobuf
    e844e5c dev: protoc-gen-go: use standard library context (requires Go1.9) - merge 3dc8a89f965ba7bf716fd0d92b83c5da1792ab9c from golang/protobuf
    dfaf7a7 dev: protobuf: Delete makefiles, regenerate protos consistently - didnt merge everything, left Makefiles, but made the other small changes - merge 251359bf9d6712b0aefe759977c168b79d1f3a27 from golang/protobuf
    828b125 dev: protoc-gen-go: fix test - merge 2b3479d8d7175442fbfd46f4ba5c14d971aeb521 from golang/protobuf
    f5a1220 dev: protoc-gen-go: add paths=source_relative option - merge 6fb8a6f1c1f011b7fde2b40f72f46587180d8d25 from golang/protobuf
    214eb97 dev: fix golden tests for older protoc versions
    aa7e6f4 dev: protoc-gen-go: handle package import names per-file - merge 9d4962b4dc40a899c435fe1aaec48e683b4300ef from golang/protobuf
    08f8895 dev: protoc-gen-go: add test for various generation params - merge a1987161d42e479a8a593d7f66ff1be81574b1e0 from golang/protobuf
    d178c98 dev: protoc-gen-go: dont depend on input file ordering - merge f4733c73b342d1d1a07fda684e831f77f840a688 from golang/protobuf
    253b333 dev: protoc-gen-go: add more golden tests for imports - merge 15c34729da28f0a8c71325b8ee35ef19362290e6 from golang/protobuf
    100bcd0 dev: Rename generated Marshal and Unmarshal protobuf method - merge 60f8421f4063f411270d0527ca77697eb9f29f1e from golang/protobuf
    26c4e69 dev: protoc-gen-go: remove relative import in main_test.go - merge protoc-gen-go: remove relative import in main_test.go from golang/protobuf
    5f4a927 dev: protoc-gen-go: include canonical import comment - merge 1021ee9d478ac35478bd39859883102741f9c4c1 from golang/protobuf
    9c3ad97 dev: proto: avoid pointer arithmetic with invalidField - merge 649500c21ecd283d00f78859ac2f386df8ed2c96 from golang/protobuf
    e23e1bc dev: no more generated package doc - merge efae459c9350a60c1f8f503f34e3cb67f803a617 from golang/protobuf
    8b846a8 dev: Merge pull request kata-containers#520 from neild/dev.alias + Fix top-level Makefile to descend into protoc-gen-go. - merge 04869ad56b1a4eb4179ceadb8a1b787a9ce4b0a3, 8cbe6f4e7d1aa3a63d2ae35cf9e9eaa1aa4c6876, 025a21d09bcd80bade5d4d6c1fb7026dcf1056dd, f7e61e16d550efe3e664c64e8ef42b624b408643, 055d7b0dba6f8ba91c7e12e4f9cc891ba607f4e7, b322e49f0e384b8e162e1f55cdeedfbba461269c + 2c2f6de12273f767388d4f3aebf6306ae3a9c7cb from golang/protobuf
    2c90c88 upstream: add back proto.Sizer
    754b8fa dev: proto: treat bad wire types as unknown fields - merge e6af52bec88380a7a18ecc0977fa4312370a970b from golang/protobuf
    0091a58 dev: jsonpb: skip unexported or non-protobuf fields - merge 42d4f477264bec37ad9b729039d071eaedd32d9b from golang/protobuf
    504621d dev: protoc-gen-go: indicate deprecated fields in documentation - merge da3e23721ffb60cccccdef6dfaef948bce1ad9d7 from golang/protobuf
    5db6fbe dev: proto: support purego build tag - merge 9a84eb8532beb2edb9dfbd6a2d823e696b57b450 from golang/protobuf
    e1d2528 dev: proto: robustify tests that compare error messages - merge 57af8637f022e8bf7f313f6156d9873b7f5ebaba from golang/protobuf
    6026053 dev: proto: remove Proto3UnknownFields flag - merge b409cc5837a65fa96edf2a5e4f1ec2ccf0cc31a8 from golang/protobuf
    81f6217 dev: proto: reject invalid UTF-8 in strings - merge 35253352f94915c119f607b2cac4ef87bd3b085b from golang/protobuf
    4192d1c dev: proto: add logic to handle legacy message - merge 10c2d9d3cccc103717e4e5dc6c503fefc8a33dea from golang/protobuf
    9806df0 dev: jsonpb: change Marshal to trim timestamp/duration to 0 fractional digits if possible - merge f9bf3fbed3136fa83399f35204bf39644e205a30 from golang/protobuf
    a30fc23 dev: jsonpb: fix handling of illegal and negative nanoseconds - merge ac606b176499a528828d10c85583a7c3107939f6 from golang/protobuf
    b34bdd4 dev: proto: expose accessors for raw value of extensions - merge 5f34c20e59ed64239722b4215413f1ffd1efa9de from golang/protobuf
    54b14bf dev: Remove raw interface - merge 7d76aa1a8129e37aae7c421a64e04a4ced5ef1ac from golang/protobuf
    5f21c7a dev: Use fmt.Errorf instead of errors.New - merge 572071ce41835e834277d132bd34f72baa4754cc from golang/protobuf
    5028789 dev: jsonpb: change Marshal/Unmarshal to return error if any required field is not set - merge 2bc5431dca4a5134e05a24d7b874cd189e934a38 from golang/protobuf
    b56d376 dev: Cleanup comments and whitespace - merge 575152efd80e5accf3969091e05f9ec30b35a2f2 from golang/protobuf
    761ef94 dev: jsonpb: check for nil in Marshal and return error to avoid panic. - merge 49f2ba7d08e875af9b5f3bd5d2f29d5fb1ca86b1 from golang/protobuf
    88bd217 dev: Simplify code - merge 5c7dd3329b568cef186709cadf093cad82f8fdfc from golang/protobuf
    d2459a7 dev: Fix uint64->int overflow in table unmarshal - merge 1ec9e17d4d187ddb55cc9858887b2202b3f75707 from golang/protobuf
    b559abf dev: Correct some mistakes - merge 013f295b1c740bd8ca5ce84ea810940b1945fcb0 from golang/protobuf
    5ec47c3 dev: Remove unused code - merge 3ffccb49d84ed0b9eb1e518dd391a6d015adfeb4 from golang/protobuf
    44af720 Upstream (kata-containers#399): Merging upstream from golang/protobuf into dev branch golang/protobuf@8cc9e46
    1ef32a8 messagename
    49944b4 grpc error usage article
    58efb20 add mentions from Johan Brandhorst
    1d2310f merge bbd03ef6da3a115852eaf24c8a1c46aeb39aa175 from golang/protobuf
    ac06767 upgrade to go1.10
    9b87cea fix build for gopherjs that now requires go 1.10
    a74c03e fix build
    1ae71f0 fix for issue 389: importing of customtypes that are messages should not cause another import for the original message
    d5bc08a Update Readme.md
    f8f204f add new user : go-spacemesh
    b75782e protoc-min-version don't suppress protoc stdout and stderr out on success. (kata-containers#381)
    73bcffa Update Readme.md
    43a6153 More well known types (kata-containers#378)
    ff3a3b6 Added link to new blog post in README (kata-containers#375)
    1adfc12 merge 925541529c1fa6821df4e44ce2723319eb2be768 from golang/protobuf
    26de2f9 Added instructions for using proto files from google/protobuf (kata-containers#371)
    160de10 another user: zero stor
    d4d8b59 Update to protobuf 3.5.1 and minor cleanups for Golang 1.10 (kata-containers#363)
    35b81a0 Test with latest Go and protobuf patch versions. (kata-containers#352)
    aee20e7 plugin/equal: "return this == nil" to satisfy gosimple linter
    620da83 plugin/populate: avoid loops on non top-level packages
    cc007c0 example
    cd5e432 Incorporate review comments
    c4bc39e Mentioning the bit about proto_path up front
    ff2773e Ensure v3 to compile importduplicate.proto
    e683811 Minor update to the comment.
    bf3b9f4 Fix issue when provided f!=nil
    563235a Fix duplicate import names when same name used in different contexts
    3813b83 fix for testdata/my_test
    685a9b3 Test duplicate import names from proto package and generated code.
    b0a8a05 Removed unsafe stuff that got readded; regenerated .pb.go files
    8ddeac9 Update generated test files.
    c9c3a74 Update gogoproto/Makefile.
    f646b88 Review feedback
    b1c3a66 Add go_package
    7eaf46c min version 3 for my_test/test.proto
    3db9d03 Grammar Issues
    0c5dcd7 Update extensions.md
    acc574d merged 130e6b02ab059e7b717a096f397c5b60111cae74 from golang/protobuf
    2e9fe32 updated descriptor
    7cc42a6 merged 11b8df160996e00fd4b55cbaafb3d84ec6d50fa8 from golang/protobuf
    04380c3 merged 17ce1425424ab154092bbb43af630bd647f3bb0d from golang/protobuf
    92733a0 merged 5afd06f9d81a86d6e3bb7dc702d6bd148ea3ff23 from golang/protobuf
    79e6522 Revised comment regarding the uniqueness of package names.
    44008aa Regenerated pb.go files.
    48a47cb Fixed double import prefix issue.
    874a222 Fixed misleading comment.
    c59a8c7 Readded alias name to import statements.
    3ae4cfd Fix to avoid printing import alias when same as import path suffix.
    ca6d352 Fix to avoid long import alias names when not needed.033462 merge in golang/protobuf commit 3a3da3a4e26776cc22a79ef46d5d58477532dede - proto: mention field name in error message (kata-containers#616) (kata-containers#465)
    98f6aa8 added license details to Readme.md (kata-containers#469)
    e66941c Update Readme.md (kata-containers#468)
    476a2e9 Set defaults on nonnullable fields (kata-containers#435) (kata-containers#459)
    5e81640 removed the GOPATH env dep in the makefile protopath (kata-containers#461)
    7af9d32 Fix nullable extension issues for non generated code (kata-containers#453)
    888d305 merge in golang/protobuf commit d7d60bc05d9f92d4692aa196ac022618c2d86655 - grpc: fix and improve interface comments for client/server types (kata-containers#604)
    a4350c5 merge in golang/protobuf commit 668a607657a5d387d6333648a2d9c749761fdc69 - LICENSE: Move title to README.md
    8599525 merged golang/protobuf commit 927b65914520a8b7d44f5c9057611cfec6b2e2d0 - proto: adjust documentation on RequiredNotSetError (kata-containers#603)
    8e3eb24 Fix wrong build tags (kata-containers#445)
    797fbbb merged commit 32a84b27e28ab9f681f0df16160c4ef1f6587094 from golang/prb

Fixes:kata-containers#555

Signed-off-by: lifupan <lifupan@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants