[PULL 0/5] Python patches

[PULL 0/5] Python patches

[John Snow]

The following changes since commit 2b22e7540d6ab4efe82d442363e3fc900cea6584:

  Merge tag 'm68k-for-6.2-pull-request' of git://github.com/vivier/qemu-m68k into staging (2021-11-09 13:16:56 +0100)

are available in the Git repository at:

  https://gitlab.com/jsnow/qemu.git tags/python-pull-request

for you to fetch changes up to c398a241ec4138e0b995a0215dea84ca93b0384f:

  scripts/device-crash-test: hide tracebacks for QMP connect errors (2021-11-16 14:26:36 -0500)

----------------------------------------------------------------
Pull request

----------------------------------------------------------------

John Snow (5):
  python/aqmp: Fix disconnect during capabilities negotiation
  python/aqmp: fix ConnectError string method
  scripts/device-crash-test: simplify Exception handling
  scripts/device-crash-test: don't emit AQMP connection errors to stdout
  scripts/device-crash-test: hide tracebacks for QMP connect errors

 python/qemu/aqmp/protocol.py | 24 ++++++++++++++++++------
 scripts/device-crash-test    | 33 +++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 14 deletions(-)

-- 
2.31.1

[PULL 1/5] python/aqmp: Fix disconnect during capabilities negotiation

[John Snow]

If we receive ConnectionResetError (ECONNRESET) while attempting to
perform capabilities negotiation -- prior to the establishment of the
async reader/writer tasks -- the disconnect function is not aware that
we are in an error pathway.

As a result, when attempting to close the StreamWriter, we'll see the
same ConnectionResetError that caused us to initiate a disconnect in the
first place, which will cause the disconnect task itself to fail, which
emits a CRITICAL logging event.

I still don't know if there's a smarter way to check to see if an
exception received at this point is "the same" exception as the one that
caused the initial disconnect, but for now the problem can be avoided by
improving the error pathway detection in the exit path.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Message-id: 20211111143719.2162525-2-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/aqmp/protocol.py | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py
index ae1df24026..860b79512d 100644
--- a/python/qemu/aqmp/protocol.py
+++ b/python/qemu/aqmp/protocol.py
@@ -623,13 +623,21 @@ async def _bh_disconnect(self) -> None:
         def _done(task: Optional['asyncio.Future[Any]']) -> bool:
             return task is not None and task.done()
 
-        # NB: We can't rely on _bh_tasks being done() here, it may not
-        #     yet have had a chance to run and gather itself.
+        # Are we already in an error pathway? If either of the tasks are
+        # already done, or if we have no tasks but a reader/writer; we
+        # must be.
+        #
+        # NB: We can't use _bh_tasks to check for premature task
+        # completion, because it may not yet have had a chance to run
+        # and gather itself.
         tasks = tuple(filter(None, (self._writer_task, self._reader_task)))
         error_pathway = _done(self._reader_task) or _done(self._writer_task)
+        if not tasks:
+            error_pathway |= bool(self._reader) or bool(self._writer)
 
         try:
-            # Try to flush the writer, if possible:
+            # Try to flush the writer, if possible.
+            # This *may* cause an error and force us over into the error path.
             if not error_pathway:
                 await self._bh_flush_writer()
         except BaseException as err:
@@ -639,7 +647,7 @@ def _done(task: Optional['asyncio.Future[Any]']) -> bool:
             self.logger.debug("%s:\n%s\n", emsg, pretty_traceback())
             raise
         finally:
-            # Cancel any still-running tasks:
+            # Cancel any still-running tasks (Won't raise):
             if self._writer_task is not None and not self._writer_task.done():
                 self.logger.debug("Cancelling writer task.")
                 self._writer_task.cancel()
@@ -652,7 +660,7 @@ def _done(task: Optional['asyncio.Future[Any]']) -> bool:
                 self.logger.debug("Waiting for tasks to complete ...")
                 await asyncio.wait(tasks)
 
-            # Lastly, close the stream itself. (May raise):
+            # Lastly, close the stream itself. (*May raise*!):
             await self._bh_close_stream(error_pathway)
             self.logger.debug("Disconnected.")
 
-- 
2.31.1

[PULL 2/5] python/aqmp: fix ConnectError string method

[John Snow]

When ConnectError is used to wrap an Exception that was initialized
without an error message, we are treated to a traceback with a rubbish
line like this:

... ConnectError: Failed to establish session:

Correct this to use the name of an exception as a fallback message:

... ConnectError: Failed to establish session: EOFError

Better!

Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Message-id: 20211111143719.2162525-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/aqmp/protocol.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py
index 860b79512d..5190b33b13 100644
--- a/python/qemu/aqmp/protocol.py
+++ b/python/qemu/aqmp/protocol.py
@@ -79,7 +79,11 @@ def __init__(self, error_message: str, exc: Exception):
         self.exc: Exception = exc
 
     def __str__(self) -> str:
-        return f"{self.error_message}: {self.exc!s}"
+        cause = str(self.exc)
+        if not cause:
+            # If there's no error string, use the exception name.
+            cause = exception_summary(self.exc)
+        return f"{self.error_message}: {cause}"
 
 
 class StateError(AQMPError):
-- 
2.31.1

[PULL 3/5] scripts/device-crash-test: simplify Exception handling

[John Snow]

We don't need to handle KeyboardInterruptError specifically; we can
instead tighten the scope of the broad Exception handlers to only catch
"Exception", which has the effect of allowing all BaseException classes
that do not inherit from Exception to be raised through.

KeyboardInterruptError and a few other important ones are
BaseExceptions, so this does the same thing with less code.

Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Message-id: 20211111143719.2162525-4-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/device-crash-test | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 8331c057b8..d91e8616ef 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -317,9 +317,7 @@ class QemuBinaryInfo(object):
         try:
             vm.launch()
             mi['runnable'] = True
-        except KeyboardInterrupt:
-            raise
-        except:
+        except Exception:
             dbg("exception trying to run binary=%s machine=%s", self.binary, machine, exc_info=sys.exc_info())
             dbg("log: %r", vm.get_log())
             mi['runnable'] = False
@@ -360,9 +358,7 @@ def checkOneCase(args, testcase):
     exc_traceback = None
     try:
         vm.launch()
-    except KeyboardInterrupt:
-        raise
-    except:
+    except Exception:
         exc_traceback = traceback.format_exc()
         dbg("Exception while running test case")
     finally:
-- 
2.31.1

[PULL 4/5] scripts/device-crash-test: don't emit AQMP connection errors to stdout

[John Snow]

These errors are expected, so they shouldn't clog up terminal output. In
the event that they're *not* expected, we'll be seeing an awful lot more
output concerning the nature of the failure.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Message-id: 20211111143719.2162525-5-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/device-crash-test | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index d91e8616ef..49bcd61b4f 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -499,6 +499,12 @@ def main():
         lvl = logging.WARN
     logging.basicConfig(stream=sys.stdout, level=lvl, format='%(levelname)s: %(message)s')
 
+    if not args.debug:
+        # Async QMP, when in use, is chatty about connection failures.
+        # This script knowingly generates a ton of connection errors.
+        # Silence this logger.
+        logging.getLogger('qemu.aqmp.qmp_client').setLevel(logging.CRITICAL)
+
     fatal_failures = []
     wl_stats = {}
     skipped = 0
-- 
2.31.1

[PULL 5/5] scripts/device-crash-test: hide tracebacks for QMP connect errors

[John Snow]

Generally, the traceback for a connection failure is uninteresting and
all we need to know is that the connection attempt failed.

Reduce the verbosity in these cases, except when debugging.

Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Message-id: 20211111143719.2162525-6-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/device-crash-test | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 49bcd61b4f..3db0ffe5b8 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -36,6 +36,7 @@ from itertools import chain
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
 from qemu.machine import QEMUMachine
+from qemu.aqmp import ConnectError
 
 logger = logging.getLogger('device-crash-test')
 dbg = logger.debug
@@ -355,10 +356,12 @@ def checkOneCase(args, testcase):
     dbg("will launch QEMU: %s", cmdline)
     vm = QEMUMachine(binary=binary, args=args)
 
+    exc = None
     exc_traceback = None
     try:
         vm.launch()
-    except Exception:
+    except Exception as this_exc:
+        exc = this_exc
         exc_traceback = traceback.format_exc()
         dbg("Exception while running test case")
     finally:
@@ -366,8 +369,9 @@ def checkOneCase(args, testcase):
         ec = vm.exitcode()
         log = vm.get_log()
 
-    if exc_traceback is not None or ec != 0:
-        return {'exc_traceback':exc_traceback,
+    if exc is not None or ec != 0:
+        return {'exc': exc,
+                'exc_traceback':exc_traceback,
                 'exitcode':ec,
                 'log':log,
                 'testcase':testcase,
@@ -455,6 +459,17 @@ def logFailure(f, level):
     for l in f['log'].strip().split('\n'):
         logger.log(level, "log: %s", l)
     logger.log(level, "exit code: %r", f['exitcode'])
+
+    # If the Exception is merely a QMP connect error,
+    # reduce the logging level for its traceback to
+    # improve visual clarity.
+    if isinstance(f.get('exc'), ConnectError):
+        logger.log(level, "%s.%s: %s",
+                   type(f['exc']).__module__,
+                   type(f['exc']).__qualname__,
+                   str(f['exc']))
+        level = logging.DEBUG
+
     if f['exc_traceback']:
         logger.log(level, "exception:")
         for l in f['exc_traceback'].split('\n'):
-- 
2.31.1

Re: [PULL 0/5] Python patches

[Richard Henderson]

On 11/17/21 1:33 AM, John Snow wrote:
> The following changes since commit 2b22e7540d6ab4efe82d442363e3fc900cea6584:
> 
>    Merge tag 'm68k-for-6.2-pull-request' of git://github.com/vivier/qemu-m68k into staging (2021-11-09 13:16:56 +0100)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/jsnow/qemu.git tags/python-pull-request
> 
> for you to fetch changes up to c398a241ec4138e0b995a0215dea84ca93b0384f:
> 
>    scripts/device-crash-test: hide tracebacks for QMP connect errors (2021-11-16 14:26:36 -0500)
> 
> ----------------------------------------------------------------
> Pull request
> 
> ----------------------------------------------------------------
> 
> John Snow (5):
>    python/aqmp: Fix disconnect during capabilities negotiation
>    python/aqmp: fix ConnectError string method
>    scripts/device-crash-test: simplify Exception handling
>    scripts/device-crash-test: don't emit AQMP connection errors to stdout
>    scripts/device-crash-test: hide tracebacks for QMP connect errors
> 
>   python/qemu/aqmp/protocol.py | 24 ++++++++++++++++++------
>   scripts/device-crash-test    | 33 +++++++++++++++++++++++++--------
>   2 files changed, 43 insertions(+), 14 deletions(-)

Applied, thanks.

r~

Re: [PULL 0/5] Python patches

[Gerd Hoffmann]

  Hi,

>   https://gitlab.com/jsnow/qemu.git tags/python-pull-request

What is the status of the plan to upload this to pypi eventually?

thanks,
  Gerd

Re: [PULL 0/5] Python patches

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2911 bytes --]

On Wed, Nov 17, 2021 at 4:42 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
>
> >   https://gitlab.com/jsnow/qemu.git tags/python-pull-request
>
> What is the status of the plan to upload this to pypi eventually?
>
>
Thanks for asking!

The honest answer is "I'm not exactly sure", but there are a few things to
work out still. Let me use this as an opportunity to try and give you an
honest answer.
We've got four packages right now: qmp, aqmp, machine and utils.

- I don't intend to *ever* upload utils, I created that one specifically as
an in-tree package for "low quality" code that we just need as glue.
- aqmp is brand new. It was moved as the default provider for the QMP
protocol in the tree (being used by machine.py) only two weeks ago. I am
using this current RC testing phase to find any problems with it.
- qmp is something I want to deprecate, I don't intend to upload it to
PyPI. I intend to rename aqmp -> qmp and have just the one qmp package. I
can't do this until next release, and only after we are confident and happy
that aqmp is stable enough.
- machine has a few problems with it. I am reluctant to upload it in its
current form. I am actively developing a new version of it that uses the
new Async QMP module. However, this might take a bit of time, I fear.

So, I think I have this timeline for myself:

- Fix bugs in AQMP package revealed during RC testing
- Introduce sync wrapper for AQMP that resembles the native AQMP interface
more than it resembles the "legacy QMP" interface.
- Remove all QEMU source tree uses of qemu.qmp and qemu.aqmp.legacy.
- Delete qemu.qmp and rename qemu.aqmp to qemu.qmp.
- Split python/qemu/qmp out into its own repository and begin uploading it
to PyPI, as a test. (Do not delete python/qemu/qmp yet at this phase.)
- Transition any users of the Python packages in the QEMU source tree to
installing the QMP dependency from PyPI instead of grabbing it from the
tree.
- Delete python/qemu/qmp from the QEMU source tree at this moment;
"re-fork" the package if necessary to collect any commits since the "test
split" procedure.


Some questions to work out:
- What tools should be uploaded with qemu.qmp? a version of qmp-shell is
high on the list for me. qom, qom-set, qom-get, qom-list, qom-tree,
qom-fuse etc I am suspecting might be better left behind in qemu.utils
instead, though. I am not sure I want to support those more broadly. They
weren't designed for "external consumption".
- qemu-ga-client should be moved over into utils, or possibly even deleted
-- it hasn't seen a lot of love and I doubt there are any users. I don't
have the bandwidth to refurbish it for no users. Maybe if there's a demand
in the future ...


... This might be being overcautious, though. Perhaps I can upload a
version of "qemu.aqmp" even this week just as a demonstration of how it
would work.

Happy to take suggestions/feedback on this process.

--js

[-- Attachment #2: Type: text/html, Size: 3865 bytes --]

Re: [PULL 0/5] Python patches

[Vladimir Sementsov-Ogievskiy]

17.11.2021 20:56, John Snow wrote:
> 
> On Wed, Nov 17, 2021 at 4:42 AM Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>> wrote:
> 
>        Hi,
> 
>      > https://gitlab.com/jsnow/qemu.git <https://gitlab.com/jsnow/qemu.git> tags/python-pull-request
> 
>     What is the status of the plan to upload this to pypi eventually?
> 
> 
> Thanks for asking!
> 
> The honest answer is "I'm not exactly sure", but there are a few things to work out still. Let me use this as an opportunity to try and give you an honest answer.
> We've got four packages right now: qmp, aqmp, machine and utils.
> 
> - I don't intend to *ever* upload utils, I created that one specifically as an in-tree package for "low quality" code that we just need as glue.
> - aqmp is brand new. It was moved as the default provider for the QMP protocol in the tree (being used by machine.py) only two weeks ago. I am using this current RC testing phase to find any problems with it.
> - qmp is something I want to deprecate, I don't intend to upload it to PyPI. I intend to rename aqmp -> qmp and have just the one qmp package. I can't do this until next release, and only after we are confident and happy that aqmp is stable enough.
> - machine has a few problems with it. I am reluctant to upload it in its current form. I am actively developing a new version of it that uses the new Async QMP module. However, this might take a bit of time, I fear.
> 
> So, I think I have this timeline for myself:
> 
> - Fix bugs in AQMP package revealed during RC testing
> - Introduce sync wrapper for AQMP that resembles the native AQMP interface more than it resembles the "legacy QMP" interface.
> - Remove all QEMU source tree uses of qemu.qmp and qemu.aqmp.legacy.
> - Delete qemu.qmp and rename qemu.aqmp to qemu.qmp.
> - Split python/qemu/qmp out into its own repository and begin uploading it to PyPI, as a test. (Do not delete python/qemu/qmp yet at this phase.)
> - Transition any users of the Python packages in the QEMU source tree to installing the QMP dependency from PyPI instead of grabbing it from the tree.
> - Delete python/qemu/qmp from the QEMU source tree at this moment; "re-fork" the package if necessary to collect any commits since the "test split" procedure.
> 

That all sounds great!

> 
> Some questions to work out:
> - What tools should be uploaded with qemu.qmp? a version of qmp-shell is high on the list for me. qom, qom-set, qom-get, qom-list, qom-tree, qom-fuse etc I am suspecting might be better left behind in qemu.utils instead, though. I am not sure I want to support those more broadly. They weren't designed for "external consumption".
> - qemu-ga-client should be moved over into utils, or possibly even deleted -- it hasn't seen a lot of love and I doubt there are any users. I don't have the bandwidth to refurbish it for no users. Maybe if there's a demand in the future ...
> 
> 
> ... This might be being overcautious, though. Perhaps I can upload a version of "qemu.aqmp" even this week just as a demonstration of how it would work.
> 

Why do we need wait for next release for renaming aqmp -> qmp? Or what next release do you mean? I think you can rename it as soon as 6.3 development phase is open.

I'm not sure that's a good idea to upload qemu.aqmp to public and than rename it to qemu.qmp.. Maybe, you can upload it now as qemu.qmp? So, first, create a separate repo with aqmp (already renamed to qmp), upload it to PyPI (as qmp) - this all as a first step. And then gradually move Qemu to use this new repo instead its own qmp/aqmp.

-- 
Best regards,
Vladimir

Re: [PULL 0/5] Python patches

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 5191 bytes --]

On Wed, Nov 17, 2021 at 1:20 PM Vladimir Sementsov-Ogievskiy <
vsementsov@virtuozzo.com> wrote:

> 17.11.2021 20:56, John Snow wrote:
> >
> > On Wed, Nov 17, 2021 at 4:42 AM Gerd Hoffmann <kraxel@redhat.com
> <mailto:kraxel@redhat.com>> wrote:
> >
> >        Hi,
> >
> >      > https://gitlab.com/jsnow/qemu.git <
> https://gitlab.com/jsnow/qemu.git> tags/python-pull-request
> >
> >     What is the status of the plan to upload this to pypi eventually?
> >
> >
> > Thanks for asking!
> >
> > The honest answer is "I'm not exactly sure", but there are a few things
> to work out still. Let me use this as an opportunity to try and give you an
> honest answer.
> > We've got four packages right now: qmp, aqmp, machine and utils.
> >
> > - I don't intend to *ever* upload utils, I created that one specifically
> as an in-tree package for "low quality" code that we just need as glue.
> > - aqmp is brand new. It was moved as the default provider for the QMP
> protocol in the tree (being used by machine.py) only two weeks ago. I am
> using this current RC testing phase to find any problems with it.
> > - qmp is something I want to deprecate, I don't intend to upload it to
> PyPI. I intend to rename aqmp -> qmp and have just the one qmp package. I
> can't do this until next release, and only after we are confident and happy
> that aqmp is stable enough.
> > - machine has a few problems with it. I am reluctant to upload it in its
> current form. I am actively developing a new version of it that uses the
> new Async QMP module. However, this might take a bit of time, I fear.
> >
> > So, I think I have this timeline for myself:
> >
> > - Fix bugs in AQMP package revealed during RC testing
> > - Introduce sync wrapper for AQMP that resembles the native AQMP
> interface more than it resembles the "legacy QMP" interface.
> > - Remove all QEMU source tree uses of qemu.qmp and qemu.aqmp.legacy.
> > - Delete qemu.qmp and rename qemu.aqmp to qemu.qmp.
> > - Split python/qemu/qmp out into its own repository and begin uploading
> it to PyPI, as a test. (Do not delete python/qemu/qmp yet at this phase.)
> > - Transition any users of the Python packages in the QEMU source tree to
> installing the QMP dependency from PyPI instead of grabbing it from the
> tree.
> > - Delete python/qemu/qmp from the QEMU source tree at this moment;
> "re-fork" the package if necessary to collect any commits since the "test
> split" procedure.
> >
>
> That all sounds great!
>
> >
> > Some questions to work out:
> > - What tools should be uploaded with qemu.qmp? a version of qmp-shell is
> high on the list for me. qom, qom-set, qom-get, qom-list, qom-tree,
> qom-fuse etc I am suspecting might be better left behind in qemu.utils
> instead, though. I am not sure I want to support those more broadly. They
> weren't designed for "external consumption".
> > - qemu-ga-client should be moved over into utils, or possibly even
> deleted -- it hasn't seen a lot of love and I doubt there are any users. I
> don't have the bandwidth to refurbish it for no users. Maybe if there's a
> demand in the future ...
> >
> >
> > ... This might be being overcautious, though. Perhaps I can upload a
> version of "qemu.aqmp" even this week just as a demonstration of how it
> would work.
> >
>
> Why do we need wait for next release for renaming aqmp -> qmp? Or what
> next release do you mean? I think you can rename it as soon as 6.3
> development phase is open.
>
>
I might be confused in my thinking because there's a ton of little tasks to
do, and I won't pretend I have thought extremely carefully about the
precise order in which they *have* to be done, only the order in which that
it occurs to me to do them. :)

I suppose I could do something like rename "qmp" to "legacy_qmp" in the
tree as an intermediate step and accomplish the "aqmp -> qmp" rename
sooner, but there's a lot of churn inherent to that. Since there's a lot of
churn inherent to moving users off of the old interface anyway, I figured
I'd just tackle it all at once... which I can't do until the tree re-opens
again.

I can certainly work on it in the meantime, though.

I'm not sure that's a good idea to upload qemu.aqmp to public and than
> rename it to qemu.qmp.. Maybe, you can upload it now as qemu.qmp? So,
> first, create a separate repo with aqmp (already renamed to qmp), upload it
> to PyPI (as qmp) - this all as a first step. And then gradually move Qemu
> to use this new repo instead its own qmp/aqmp.
>

I'm afraid of doing this because I don't want to pollute the 'qemu.qmp'
space with two packages that contain radically different things in it. It
feels safer to fully switch over the QEMU source tree first, and THEN
upload to PyPI. If I go out of order there, I worry that we will run into
circumstances where various scripts/tools will use "the wrong qemu.qmp",
and it will be frustrating for people without a lot of Python packaging
experience to diagnose on their own.

The safest thing is just to wait for me to do all the cleanup churn I laid
out above, but if I were to demo a "preview", doing it under the
'qemu.aqmp' name seems like the safest bet because I don't plan to use that
name long-term.

--js

[-- Attachment #2: Type: text/html, Size: 6516 bytes --]

Re: [PULL 0/5] Python patches

[Vladimir Sementsov-Ogievskiy]

17.11.2021 22:07, John Snow wrote:
> 
> 
> On Wed, Nov 17, 2021 at 1:20 PM Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com <mailto:vsementsov@virtuozzo.com>> wrote:
> 
>     17.11.2021 20:56, John Snow wrote:
>      >
>      > On Wed, Nov 17, 2021 at 4:42 AM Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com> <mailto:kraxel@redhat.com <mailto:kraxel@redhat.com>>> wrote:
>      >
>      >        Hi,
>      >
>      >      > https://gitlab.com/jsnow/qemu.git <https://gitlab.com/jsnow/qemu.git> <https://gitlab.com/jsnow/qemu.git <https://gitlab.com/jsnow/qemu.git>> tags/python-pull-request
>      >
>      >     What is the status of the plan to upload this to pypi eventually?
>      >
>      >
>      > Thanks for asking!
>      >
>      > The honest answer is "I'm not exactly sure", but there are a few things to work out still. Let me use this as an opportunity to try and give you an honest answer.
>      > We've got four packages right now: qmp, aqmp, machine and utils.
>      >
>      > - I don't intend to *ever* upload utils, I created that one specifically as an in-tree package for "low quality" code that we just need as glue.
>      > - aqmp is brand new. It was moved as the default provider for the QMP protocol in the tree (being used by machine.py) only two weeks ago. I am using this current RC testing phase to find any problems with it.
>      > - qmp is something I want to deprecate, I don't intend to upload it to PyPI. I intend to rename aqmp -> qmp and have just the one qmp package. I can't do this until next release, and only after we are confident and happy that aqmp is stable enough.
>      > - machine has a few problems with it. I am reluctant to upload it in its current form. I am actively developing a new version of it that uses the new Async QMP module. However, this might take a bit of time, I fear.
>      >
>      > So, I think I have this timeline for myself:
>      >
>      > - Fix bugs in AQMP package revealed during RC testing
>      > - Introduce sync wrapper for AQMP that resembles the native AQMP interface more than it resembles the "legacy QMP" interface.
>      > - Remove all QEMU source tree uses of qemu.qmp and qemu.aqmp.legacy.
>      > - Delete qemu.qmp and rename qemu.aqmp to qemu.qmp.
>      > - Split python/qemu/qmp out into its own repository and begin uploading it to PyPI, as a test. (Do not delete python/qemu/qmp yet at this phase.)
>      > - Transition any users of the Python packages in the QEMU source tree to installing the QMP dependency from PyPI instead of grabbing it from the tree.
>      > - Delete python/qemu/qmp from the QEMU source tree at this moment; "re-fork" the package if necessary to collect any commits since the "test split" procedure.
>      >
> 
>     That all sounds great!
> 
>      >
>      > Some questions to work out:
>      > - What tools should be uploaded with qemu.qmp? a version of qmp-shell is high on the list for me. qom, qom-set, qom-get, qom-list, qom-tree, qom-fuse etc I am suspecting might be better left behind in qemu.utils instead, though. I am not sure I want to support those more broadly. They weren't designed for "external consumption".
>      > - qemu-ga-client should be moved over into utils, or possibly even deleted -- it hasn't seen a lot of love and I doubt there are any users. I don't have the bandwidth to refurbish it for no users. Maybe if there's a demand in the future ...
>      >
>      >
>      > ... This might be being overcautious, though. Perhaps I can upload a version of "qemu.aqmp" even this week just as a demonstration of how it would work.
>      >
> 
>     Why do we need wait for next release for renaming aqmp -> qmp? Or what next release do you mean? I think you can rename it as soon as 6.3 development phase is open.
> 
> 
> I might be confused in my thinking because there's a ton of little tasks to do, and I won't pretend I have thought extremely carefully about the precise order in which they *have* to be done, only the order in which that it occurs to me to do them. :)
> 
> I suppose I could do something like rename "qmp" to "legacy_qmp" in the tree as an intermediate step and accomplish the "aqmp -> qmp" rename sooner, but there's a lot of churn inherent to that. Since there's a lot of churn inherent to moving users off of the old interface anyway, I figured I'd just tackle it all at once... which I can't do until the tree re-opens again.
> 
> I can certainly work on it in the meantime, though.
> 
>     I'm not sure that's a good idea to upload qemu.aqmp to public and than rename it to qemu.qmp.. Maybe, you can upload it now as qemu.qmp? So, first, create a separate repo with aqmp (already renamed to qmp), upload it to PyPI (as qmp) - this all as a first step. And then gradually move Qemu to use this new repo instead its own qmp/aqmp.
> 
> 
> I'm afraid of doing this because I don't want to pollute the 'qemu.qmp' space with two packages that contain radically different things in it. It feels safer to fully switch over the QEMU source tree first, and THEN upload to PyPI. If I go out of order there, I worry that we will run into circumstances where various scripts/tools will use "the wrong qemu.qmp", and it will be frustrating for people without a lot of Python packaging experience to diagnose on their own.

Agree

> 
> The safest thing is just to wait for me to do all the cleanup churn I laid out above, but if I were to demo a "preview", doing it under the 'qemu.aqmp' name seems like the safest bet because I don't plan to use that name long-term.
> 

OK, that's right. I just imagined a "fast path", but it's not safe, you are right.


-- 
Best regards,
Vladimir

Re: [PULL 0/5] Python patches

[Gerd Hoffmann]

  Hi,

> - Split python/qemu/qmp out into its own repository and begin uploading it
> to PyPI, as a test. (Do not delete python/qemu/qmp yet at this phase.)

I think you can do that as two separate steps.

pip can install from vcs too, i.e. when splitted to a separate repo but
not yet uploaded to pypi you can simply drop something like ...

	git+https://gitlab.com/qemu/qemu-python.git@master

... into pip-requirements.txt.  That way you can easily test things
before actually uploading to pypi.

take care,
  Gerd

Re: [PULL 0/5] Python patches

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Thu, Nov 18, 2021 at 1:46 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
>
> > - Split python/qemu/qmp out into its own repository and begin uploading
> it
> > to PyPI, as a test. (Do not delete python/qemu/qmp yet at this phase.)
>
> I think you can do that as two separate steps.
>
> pip can install from vcs too, i.e. when splitted to a separate repo but
> not yet uploaded to pypi you can simply drop something like ...
>
>         git+https://gitlab.com/qemu/qemu-python.git@master
>
> ... into pip-requirements.txt.  That way you can easily test things
> before actually uploading to pypi.
>
>
Indeed - a limitation here however is that pip will not install from this
source unless explicitly asked to, so you couldn't use this package as a
requirement for another one, for example -- but it works as a testing step.
but that's the rough outline of where I am headed and what I think needs to
be done to get there. It's just taking me a while to get everything put in
order exactly the right way to be able to flip the switch. Hopefully soon,
though.

I realized when re-reading my mails last night that I said I wouldn't be
able to do it until "next release" but what I really meant was "until the
next development window".

--js

[-- Attachment #2: Type: text/html, Size: 1811 bytes --]