Simplify "cpuinfo" subcommand check#2633
Simplify "cpuinfo" subcommand check#2633lianakoleva wants to merge 6 commits intocheckpoint-restore:criu-devfrom
Conversation
The command "cpuinfo" requires a subcommand "dump" or "check". - `CR_CPUINFO` is removed; `CR_CPUINFO_DUMP` and `CR_CPUINFO_CHECK` are added in its place. - All parsing is now done in `parse_criu_mode`, removing need for passing string subcommand to `image_dir_mode`. - Check for valid subcommand is done once at the start with only the enum `CR_CPUINFO_DUMP` or `CR_CPUINFO_CHECK` propagated on success, rather than propagating both `CR_CPUINFO` and the subcommand string while doing `strcmp` on each call to `image_dir_mode`. Signed-off-by: Liana Koleva <43767763+lianakoleva@users.noreply.github.com>
How does it affect existing users who use CR_CPUINFO? |
|
@avagin Could you share an example? |
|
@lianakoleva I think I messed up when I was trying to read this code last time. CR_CPUINFO is a completely internal thing and it isn't a part of our public interface. |
|
A friendly reminder that this PR had no activity for 30 days. |
Signed-off-by: Andrei Vagin <avagin@gmail.com>
|
@lianakoleva Sorry for the long delay. The main problem was that I hadn't seen a significant difference between the current and new version. Yesterday, I decided to look at this PR more closely, which led to even more cleanups in crtools.c: lianakoleva#1. Could you please review this change and merge it into your branch? |
The cpuinfo command requires a "dump" or "check" subcommand. Thus, we replace `CR_CPUINFO` with `CR_CPUINFO_DUMP` and `CR_CPUINFO_CHECK`. This allows us to remove unnecessary subcommand check in `image_dir_mode()` and perform all parsing in `parse_criu_mode()`. With this change the check for validating the cpuinfo subcommand is now done only once with `CR_CPUINFO_DUMP` or `CR_CPUINFO_CHECK` enum. Signed-off-by: Liana Koleva <43767763+lianakoleva@users.noreply.github.com> Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
The `criu cpuinfo check` command calls cpu_validate_cpuinfo(), which attempts to open the cpuinfo.img file using `open_image()`. If the image file is not found, `open_image()` returns an "empty image" object. As a result, `cpu_validate_cpuinfo()` tries to read from it and fails with the following error: (00.002473) Error (criu/protobuf.c:72): Unexpected EOF on (empty-image) This patch adds a check for an empty image and appropriate error message. Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
|
@lianakoleva Thank you for working on this! @avagin I've updated Liana's branch with the cleanups in |
crtools: do a few minor cleanups
|
I pushed all relevant commits into the criu-dev branch. Thanks. |
3 participants
The command "cpuinfo" requires a subcommand "dump" or "check".
CR_CPUINFOis removed;CR_CPUINFO_DUMPandCR_CPUINFO_CHECKare added in its place.parse_criu_mode, removing need for passing string subcommand toimage_dir_mode.CR_CPUINFO_DUMPorCR_CPUINFO_CHECKpropagated on success, rather than propagating bothCR_CPUINFOand the subcommand string while doingstrcmpon each call toimage_dir_mode.