Skip to content
Snippets Groups Projects
Select Git revision
  • feature/libtcg-rebase default protected
1 result
Search by author
  • Any Author
  • Anton Anton anjo
1 author
  1. Oct 02, 2021
    • John Snow's avatar
      qapi/parser: enable mypy checks · 2e28283e
      John Snow authored 
      
Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>

Message-Id: <20210930205716.1148693-12-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      2e28283e
    • John Snow's avatar
      qapi/parser: Add FIXME for consolidating JSON-related types · 15acf48c
      John Snow authored 
      
The fix for this comment is forthcoming in a future commit, but this
will keep me honest. The linting configuration in ./python/setup.cfg
prohibits 'FIXME' comments. A goal of this long-running series is to
move ./scripts/qapi to ./python/qemu/qapi so that the QAPI generator is
regularly type-checked by GitLab CI.

This comment is a time-bomb to force me to address this issue prior to
that step.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-11-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      15acf48c
    • John Snow's avatar
      qapi/parser: add type hint annotations (QAPIDoc) · 5f0d9f3b
      John Snow authored 
      
Annotations do not change runtime behavior.
This commit consists of only annotations.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-10-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      5f0d9f3b
    • John Snow's avatar
      qapi/parser: add import cycle workaround · e7ac60fc
      John Snow authored 
      Adding static types causes a cycle in the QAPI generator:
[schema -> expr -> parser -> schema]. It exists because the QAPIDoc
class needs the names of types defined by the schema module, but the
schema module needs to import both expr.py/parser.py to do its actual
parsing.

Ultimately, the layering violation is that parser.py should not have any
knowledge of specifics of the Schema. QAPIDoc performs double-duty here
both as a parser *and* as a finalized object that is part of the schema.

In this patch, add the offending type hints alongside the workaround to
avoid the cycle becoming a problem at runtime. See
https://mypy.readthedocs.io/en/latest/runtime_troubles.html#import-cycles


for more information on this workaround technique.

I see three ultimate resolutions here:

(1) Just keep this patch and use the TYPE_CHECKING trick to eliminate
    the cycle which is only present during static analysis.

(2) Don't bother to annotate connect_member() et al, give them 'object'
    or 'Any'. I don't particularly like this, because it diminishes the
    usefulness of type hints for documentation purposes. Still, it's an
    extremely quick fix.

(3) Reimplement doc <--> definition correlation directly in schema.py,
    integrating doc fields directly into QAPISchemaMember and relieving
    the QAPIDoc class of the responsibility. Users of the information
    would instead visit the members first and retrieve their
    documentation instead of the inverse operation -- visiting the
    documentation and retrieving their members.

My preference is (3), but in the short-term (1) is the easiest way to
have my cake (strong type hints) and eat it too (Not have import
cycles). Do (1) for now, but plan for (3).

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-9-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      e7ac60fc
    • John Snow's avatar
      qapi/parser: Introduce NullSection · f4c05aaf
      John Snow authored 
      
Here's the weird bit. QAPIDoc generally expects -- virtually everywhere
-- that it will always have a current section. The sole exception to
this is in the case that end_comment() is called, which leaves us with
*no* section. However, in this case, we also don't expect to actually
ever mutate the comment contents ever again.

NullSection is just a Null-object that allows us to maintain the
invariant that we *always* have a current section, enforced by static
typing -- allowing us to type that field as QAPIDoc.Section instead of
the more ambiguous Optional[QAPIDoc.Section].

end_section is renamed to switch_section and now accepts as an argument
the new section to activate, clarifying that no callers ever just
unilaterally end a section; they only do so when starting a new section.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>

Message-Id: <20210930205716.1148693-8-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      f4c05aaf
    • John Snow's avatar
      qapi/parser: clarify _end_section() logic · 1e20a775
      John Snow authored 
      
The "if self._section" clause in end_section is mysterious: In which
circumstances might we end a section when we don't have one?

QAPIDoc always expects there to be a "current section", only except
after a call to end_comment(). This actually *shouldn't* ever be 'None',
so let's remove that logic so I don't wonder why it's like this again in
three months.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-7-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      1e20a775
    • John Snow's avatar
      qapi/parser: remove FIXME comment from _append_body_line · cd87c14c
      John Snow authored 
      
True, we do not check the validity of this symbol -- but we don't check
the validity of definition names during parse, either -- that happens
later, during the expr check. I don't want to introduce a dependency on
expr.py:check_name_str here and introduce a cycle.

Instead, rest assured that a documentation block is required for each
definition. This requirement uses the names of each section to ensure
that we fulfilled this requirement.

e.g., let's say that block-core.json has a comment block for
"Snapshot!Info" by accident. We'll see this error message:

In file included from ../../qapi/block.json:8:
../../qapi/block-core.json: In struct 'SnapshotInfo':
../../qapi/block-core.json:38: documentation comment is for 'Snapshot!Info'

That's a pretty decent error message.

Now, let's say that we actually mangle it twice, identically:

../../qapi/block-core.json: In struct 'Snapshot!Info':
../../qapi/block-core.json:38: struct has an invalid name

That's also pretty decent. If we forget to fix it in both places, we'll
just be back to the first error.

Therefore, let's just drop this FIXME and adjust the error message to
not imply a more thorough check than is actually performed.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-6-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      cd87c14c
    • John Snow's avatar
      qapi/parser: fix unused check_args_section arguments · 012336a1
      John Snow authored 
      
Pylint informs us we're not using these arguments. Oops, it's
right. Correct the error message and remove the remaining unused
parameter.

Fix test output now that the error message is improved.

Fixes: e151941d
Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-4-jsnow@redhat.com>
[Commit message formatting tweaked]
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      012336a1
    • John Snow's avatar
      qapi/gen: use dict.items() to iterate over _modules · 2adb988e
      John Snow authored 
      
New pylint warning. I could silence it, but this is the only occurrence
in the entire tree, including everything in iotests/ and python/. Easier
to just change this one instance.

(The warning is emitted in cases where you are fetching the values
anyway, so you may as well just take advantage of the iterator to avoid
redundant lookups.)

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-3-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      2adb988e
    • John Snow's avatar
      qapi/pylintrc: ignore 'consider-using-f-string' warning · 1c009174
      John Snow authored 
      
Pylint 2.11.x adds this warning. We're not yet ready to pursue that
conversion, so silence it for now.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-2-jsnow@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      1c009174
  3. Sep 27, 2021
    • Markus Armbruster's avatar
      qapi: Drop simple unions · 4e99f4b1
      Markus Armbruster authored 
      
Simple unions predate flat unions.  Having both complicates the QAPI
schema language and the QAPI generator.  We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.

The previous commits eliminated simple union from the tree.  Now drop
them from the QAPI schema language entirely, and update mentions of
"flat union" to just "union".

Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Message-Id: <20210917143134.412106-22-armbru@redhat.com>
      4e99f4b1
  5. Sep 25, 2021
  7. Sep 15, 2021
  9. Sep 08, 2021
  11. Sep 06, 2021
  13. Sep 03, 2021
  15. Sep 01, 2021
    • Alexander Bulekov's avatar
      fuzz: add an instrumentation filter · dfc86c0f
      Alexander Bulekov authored 
      
By default, -fsanitize=fuzzer instruments all code with coverage
information. However, this means that libfuzzer will track coverage over
hundreds of source files that are unrelated to virtual-devices. This
means that libfuzzer will optimize inputs for coverage observed in timer
code, memory APIs etc. This slows down the fuzzer and stores many inputs
that are not relevant to the actual virtual-devices.

With this change, clang versions that support the
"-fsanitize-coverage-allowlist" will only instrument a subset of the
compiled code, that is directly related to virtual-devices.

Signed-off-by: default avatarAlexander Bulekov <alxndr@bu.edu>
Reviewed-by: default avatarDarren Kenny <darren.kenny@oracle.com>
      dfc86c0f
  17. Aug 26, 2021
  19. Aug 11, 2021
  21. Jul 30, 2021